feat(search): s2 migration
Description
I believe this is blocked until we get the S2 icons.
S2 migration of Search to align with design specs. On top of the migration work, this PR includes:
- Removal of /themes
- Removal of quiet variant
- Increased Chromatic coverage
- Minor mod adjustments
- Updated migration guide notes
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
- [ ] Storybook testing preview shows all variants (except for hover as we cant show that without adding a class)
- [ ] Test WHCM in light and dark mode
- [ ] Be sure to test hover while focused since there are specific tokens applied for this
- [ ] Design validation
Regression testing
Validate:
- The documentation pages for at least two other components are still loading, including:
- [ ] The pages render correctly, are accessible, and are responsive.
- If components have been modified, VRTs have been run on this branch:
- [ ] VRTs have been run and looked at.
- [ ] Any VRT changes have been accepted (by reviewer and/or PR author), or there are no changes.
To-do list
- [x] I have read the contribution guidelines.
- [x] I have updated relevant storybook stories and templates.
- [x] I have tested these changes in Windows High Contrast mode.
- [x] If my change impacts documentation, I have updated the documentation accordingly.
- [ ] ✨ This pull request is ready to merge. ✨
🦋 Changeset detected
Latest commit: ebc11c5f336296c50c4ccfd309484d5e4e2c98a1
The changes in this PR will be included in the next version bump.
This PR includes changesets to release 3 packages
| Name | Type |
|---|---|
| @spectrum-css/tokens | Patch |
| @spectrum-css/search | Major |
| @spectrum-css/preview | Patch |
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
🚀 Deployed on https://pr-2673--spectrum-css.netlify.app
File metrics
Summary
Total size: 1.37 MB* Total change (Δ): 🔴 ⬆ 0.06 KB (0.00%)
Table reports on changes to a package's main file. Other changes can be found in the collapsed Details section below.
| Package | Size | Minified | Gzipped | Δ |
|---|---|---|---|---|
| search | 12.46 KB | 11.95 KB | 1.98 KB | 🔴 ⬆ 1.29 KB |
File change details
search
| Filename | Head | Minified | Gzipped | Compared to base |
|---|---|---|---|---|
| index.css | 12.46 KB | 11.95 KB | 1.98 KB | 🔴 ⬆ 1.29 KB |
| metadata.json | 7.34 KB | - | - | 🔴 ⬆ 0.60 KB |
tokens
| Filename | Head | Minified | Gzipped | Compared to base |
|---|---|---|---|---|
| css/dark-vars.css | 46.44 KB | - | - | - |
| css/global-vars.css | 69.59 KB | - | - | 🔴 ⬆ 0.03 KB |
| css/index.css | 241.80 KB | - | - | 🔴 ⬆ 0.03 KB |
| css/large-vars.css | 40.81 KB | - | - | - |
| css/light-vars.css | 46.43 KB | - | - | - |
| css/medium-vars.css | 40.93 KB | - | - | - |
| json/tokens.json | 380.07 KB | - | - | - |
* An ASCII character in UTF-8 is 8 bits or 1 byte.
Thanks for the feedback! Some responses for Rise:
- Disabled border color - Great callout. I've updated Textfield to improve this which will probably just be a patch change.
- .is-keyboardFocused:hover - I've also added this, good catch.
- Help text - Yeah totally! I added some help text sections to our Chromatic coverage, and you should see a control to show the help text and also the ability to change the text now.
- Clear button - I went down a bit of a rabbit hole with this one trying to use
useArgs()to only show this button when there's an input value. I wasn't able to get it working within a reasonable timebox and came to the same conclusion you did about the spacing. - WHCM placeholder color - I noticed this too. I think that using the disabled color for the placeholder text can help communicate that its placeholder text, so that seemed ok to me. Let me know if you have a different value you think would work better!
And a response for Josh:
- Clear button focus - This feels like it's in that gray area of whether we would handle this or if it's an implementation detail 🤔 Seems like removing the focus on our end would be a Storybook-only adjustment and would look something like adding an arg to ClearButton to adjust aria-hidden and tab-index, is that what you were thinking? I can add a commit for that and see what you think.
- Last thing - what is the minimised variant? I assume it's just like a search icon that you click on and then the search field appears? Do you think we'd need to do anything with that?
@rise-erpelding I have CSS-1158 written up for the minimized variant. When I last heard from Kami and Lynn, the file/branch for that variant was corrupted so Kami was in the midst of recreating I believe. The minimized search is basically just a search icon-only button, and when clicked, would expand the field, if I remember correctly. I can share Slack threads in a DM if you'd like!
- Last thing - what is the minimised variant? I assume it's just like a search icon that you click on and then the search field appears? Do you think we'd need to do anything with that?
@rise-erpelding I have CSS-1158 written up for the minimized variant. When I last heard from Kami and Lynn, the file/branch for that variant was corrupted so Kami was in the midst of recreating I believe. The minimized search is basically just a search icon-only button, and when clicked, would expand the field, if I remember correctly. I can share Slack threads in a DM if you'd like!
I did see that ticket at the last Backlog review, thanks!! No further details needed for now 🙂