SearchField: Add onKeyUp prop #2326
Conversation
🦋 Changeset detectedLatest commit: dd2759e 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 |
GeraldRequired Reviewers
Don't want to be involved in this pull request? Comment |
npm Snapshot: Published🎉 Good news!! We've packaged up the latest commit from this PR (0d070d7) and published all packages with changesets to npm. You can install the packages in webapp by running: ./services/static/dev/tools/deploy_wonder_blocks.js --tag="PR2326"Packages can also be installed manually by running: yarn add @khanacademy/wonder-blocks-<package-name>@PR2326 |
|
Size Change: 0 B Total Size: 100 kB ℹ️ View Unchanged
|
A new build was pushed to Chromatic! 🚀https://5e1bf4b385e3fb0020b7073c-zotfbkoier.chromatic.com/ Chromatic results:
|
| @@ -1,6 +1,5 @@ | |||
| export default { | |||
| clearAriaLabel: { | |||
| description: `ARIA label for the clear button. Defaults to "Clear search".`, | |||
There was a problem hiding this comment.
praise: Thanks for cleaning this up!
I will also start removing descriptions from story argtypes as I go along so we can rely on the prop docs to avoid duplication 😄
| /** | ||
| * Called when a key is released. | ||
| */ | ||
| onKeyUp?: (event: React.KeyboardEvent<HTMLInputElement>) => unknown; |
There was a problem hiding this comment.
question: I noticed we often deconstruct the specific props before applying it to the underlying element. Do we do that so that the remaining otherProps will usually be the aria props? Are there any drawbacks to letting other props be part of the otherProps?
Here's an example where we do that with the other event handlers:
There was a problem hiding this comment.
That's correct, we use that for aria props, and also for letting other valid attributes be injected in the inner node. I don't think there are drawbacks with that logic.... the only thing is that TS would complain, but this can be fixed by adding the missing props, like the one included here :).
Summary:
The SearchField component now accepts an
onKeyUpprop that will be called whenthe user releases a key while the input is focused.
This is to bring consistency with the other input components in the library and
also to allow for more flexibility when using the SearchField component.
Also took the opportunity to clear some of the SB argtype descriptions, as we no
longer need to duplicate them as of SB 8.0.
NOTE: This will be used as part of the Search page refactor.
Issue: FEI-5728
Test plan:
Verify that the unit tests pass.