Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SuperSelect - docs for search position (HDS-4254) #2620

Merged
merged 21 commits into from
Jan 23, 2025

Conversation

KristinLBradley
Copy link
Contributor

@KristinLBradley KristinLBradley commented Dec 19, 2024

📌 Summary

If merged, this PR adds documentation for the SuperSelect searchFieldPosition feature.

Related:

🔗 External links


👀 Component checklist

  • [ ] Percy was checked for any visual regression
  • [ ] A changelog entry was added via Changesets if needed (see templates here)

💬 Please consider using conventional comments when reviewing this PR.

Copy link

vercel bot commented Dec 19, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
hds-showcase ✅ Ready (Inspect) Visit Preview Jan 23, 2025 7:05pm
hds-website ✅ Ready (Inspect) Visit Preview Jan 23, 2025 7:05pm

@hashibot-hds hashibot-hds added the docs-website Content updates to the documentation website label Dec 19, 2024
@KristinLBradley KristinLBradley marked this pull request as ready for review December 19, 2024 18:57
@KristinLBradley KristinLBradley requested review from a team as code owners December 19, 2024 18:57
@KristinLBradley
Copy link
Contributor Author

KristinLBradley commented Dec 19, 2024

@majedelass Do you think any of the illustrations or design related docs need updating? I didn't see anything obvious when I looked through but wanted to double-check.

Web docs preview: https://hds-website-git-hds-4254-super-select-search-p-2680bc-hashicorp.vercel.app/components/form/super-select

@KristinLBradley KristinLBradley marked this pull request as draft December 19, 2024 23:38
@KristinLBradley

This comment was marked as resolved.

@Dhaulagiri
Copy link
Collaborator

I would think 4.16

@jorytindall
Copy link
Contributor

I went down the rabbit hole with #2612 to better understand where this content was coming from, and I personally found it a bit confusing to read through. Given the context it looks like we're documenting another argument along with the rest of the component API, but the note about it not being possible to override kind of makes this prop useless to consumers. That is, unless we determine that it's useful for consumers to know that we are setting this argument explicitly for accessibility reasons being that it's coming from PowerSelect.

At a minimum it might be helpful to make that more clear immediately in the component API (leading with the content about not being able to override it and why), then capturing the rest of the details about the property if necessary.

Not overly opinionated on the topic and I don't have all of the background, just noting a couple of thoughts.

@jorytindall
Copy link
Contributor

I removed documentation for renderInPlace and ``searchFieldPosition` within the API docs properties list and added mention of them within an existing related Info banner.

@KristinLBradley personally, I think this is a better solution than including them in the component API list which seemed to create a bit of a false equivalency. Thanks for making this change, curious what other folks think!

shleewhite
shleewhite previously approved these changes Jan 8, 2025
@Dhaulagiri Dhaulagiri added this to the [email protected] milestone Jan 9, 2025
majedelass
majedelass previously approved these changes Jan 13, 2025
Copy link
Contributor

@majedelass majedelass left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good. I removed the warning within the guidelines about the nested interactive of the previous version.

# Conflicts:
#	website/docs/components/form/super-select/index.md
#	yarn.lock
alex-ju
alex-ju previously approved these changes Jan 23, 2025
shleewhite
shleewhite previously approved these changes Jan 23, 2025
Copy link
Contributor

@shleewhite shleewhite left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

website changes look good! But why are there changes to yarn.lock?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the new changelog scripts I think these updates can be removed, as they'll be generated later.

@KristinLBradley KristinLBradley dismissed stale reviews from shleewhite and alex-ju via aea2fac January 23, 2025 19:00
@KristinLBradley KristinLBradley merged commit 0b80723 into main Jan 23, 2025
10 checks passed
@KristinLBradley KristinLBradley deleted the hds-4254-super-select-search-position-docs branch January 23, 2025 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs-website Content updates to the documentation website
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants