feat(search): collapsed search #4115
Conversation
🦋 Changeset detectedLatest commit: 62498f8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
File metricsSummaryTotal size: 1.43 MB*
search
* An ASCII character in UTF-8 is 8 bits or 1 byte. |
📚 Branch previewPR #4115 has been deployed to Azure Blob Storage: https://spectrumcss.z13.web.core.windows.net/pr-4115/index.html. |
There was a problem hiding this comment.
Nice work on this. I feel like I still mainly have questions, so I left a few for you 😊
I might consider just updating the changeset heading to "Collapsed" instead of "Minimized." Just personal preference though.
Have you reached out to design to ask about some of the focus and keyboard focus behaviors? I found the collapsed usage section in Figma, and the "focus state" of this button is just...open? That feels odd to me for some reason. Like if I was tabbing along, does it make sense for me to tab to this action button, and then it just be a completely different component? Anyways, I think this is more along the lines that I would expect, where I'm tabbing along, and I have to trigger the rest of the search field to open. I may be wrong though. Is that worth bringing up with design?
|
|
||
| /* Animation for collapsible search expansion */ | ||
| &.is-collapsed { | ||
| transition: inline-size 0.3s cubic-bezier(0.4, 0, 0.2, 1); |
There was a problem hiding this comment.
how would you feel about maybe using of any of the --spectrum-animation-duration-* tokens? Maybe we can use that here? Or was the 0.3 design directed? And just because I'm nosy, where did you find these timing curves?!
Should the cubic-beziers and even the transition-duration be set as custom props? Maybe it's not necessary, but we do use the same curve & durations twice 🤷♀️
There was a problem hiding this comment.
I'm just experimenting with animation curves here. I don't know what the intended animation should be
There was a problem hiding this comment.
oh nice! I've been learning about each of the values of the cubic-bezier function, so I was just curious!
| customClasses: [ | ||
| `${rootClass}-actionButton`, | ||
| isHovered && "is-hover", | ||
| isDisabled && "is-disabled", |
There was a problem hiding this comment.
My only suggestion is to make sure we can't keyboard focus to this button when it's disabled!
I'm not sure if this is a property thing? It certainly looks like we're passing the disabled arg through correctly- I see it on the search AND on the action button, but maybe we need to revisit the disabled+keyboard focus styles.
marissahuysentruyt
left a comment
There was a problem hiding this comment.
beautiful!
Not technically a blocker, but we should make sure to continue defining a custom prop to use in our styles for consistency and maintainability. Instead of using --spectrum-animation-duration-800 directly in the styles, we typically create a component-specific custom prop that is set to animation-duration-800. It would help in the long run if that spec needs to change, we'd only have to update it once in the custom props at the top of the file, instead of multiple times throughout the file.
Again, not technically a blocker, but I'm a sucker for consistency 🥴
Description
The search component allows for a minimized state where the search field is collapsed to an action button. When clicked the the search field is visible.
How and where has this been tested?
Please tag yourself on the tests you've marked complete to confirm the tests have been run by someone other than the author.
Validation steps
Include steps for the PR reviewer that explain how they should test your PR. Example test outline:
trueis collapsed state (@cdransf)Regression testing
Validate:
Screenshots
States
Accessibility
Keyboard Navigation
Tabkey to access the search button and pressEnterto expand the search fieldTo-do list