WB-1780: Refactor combobox to use Announcer [WIP] #2390
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We don't need this yet!
Why didn't Prettier do this automatically? I do not know...
1. Keeps announce from being called too frequently. We can play with the timeout duration. 2. Makes the returned IDREF more reliable in a browser.
1. Keeps announce from being called too frequently. We can play with the timeout duration. 2. Makes the returned IDREF more reliable in a browser.
1. Return first result and debounce subsequent calls within the debounceThreshold 2. Remove removalDelay parameter to simplify API 3. Put debounce utility into a separate file and add tests
|
|
Size Change: -1.33 kB (-1.34%) Total Size: 98 kB
ℹ️ View Unchanged
|
Debounce wasn't actually limiting execution in the wait period. Now it does, and the debounce duration is still configurable when calling announceMessage!
I was trying to avoid having to import the Announcer in this test to keep things isolated, but it's so specific to the Announcer that I decided it didn't matter that much. Specifying the Announcer instance for the scope instead of generic thisArg logic simplified things quite a bit as well.
marcysutton
force-pushed
the
announcer-pt1
branch
from
4957581 to
7eacfef
Compare
## Summary: This is the first step in removing our custom ID utilities in favour of using the `useId` hook from React's API. This PR is not going to be merged/released on its own. Another PR will be coming to add an `Id` component that wraps `useId` so that there's an easier path for consumers to migrate. The difficult bit is moving from the provider architecture that gives an ID factory, to the `useId`/`Id` approach. However, long term, it should simplify things greatly. ### Upgrade note: Upgrading to this will cause deprecation notices in consuming code. If a lint rule is set up to block deprecated usage (which it probably is), then those errors will have to be suppressed. A find/replace may help in some circumstances, but this may also be a manual process. The alternative, of course, is to actually modify stuff to the new approach. Issue: WB-1812 ## Test plan: `yarn test` `yarn lint` Author: somewhatabstract Reviewers: jandrade Required Reviewers: Approved By: jandrade Checks: ✅ Chromatic - Get results on regular PRs (ubuntu-latest, 20.x), ✅ Test / Test (ubuntu-latest, 20.x, 2/2), ✅ Test / Test (ubuntu-latest, 20.x, 1/2), ✅ Lint / Lint (ubuntu-latest, 20.x), ✅ Check build sizes (ubuntu-latest, 20.x), ✅ Chromatic - Build on regular PRs / chromatic (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ⏭️ Chromatic - Skip on Release PR (changesets), ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ gerald, ⏭️ dependabot Pull Request URL: #2388
## Summary: This adds a simple CaaF (children-as-a-function) component to generate unique IDs as a stand-in for the `useId` hook. This is useful for cases where one needs to generate unique IDs for a component, but cannot use the hook without a larger refactor. Issue: WB-1812 ## Test plan: `yarn test` `yarn start:storybook` to check for the added docs `yarn typecheck` Author: somewhatabstract Reviewers: jeresig Required Reviewers: Approved By: jeresig Checks: ⌛ Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), ⌛ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ⌛ gerald, ⏭️ dependabot Pull Request URL: #2389
## Summary: This is the last piece in the first batch of work. This migrates all Wonder Blocks components off our old ID providers and onto the new `Id` component. There are also some documentation tweaks to make the deprecation clearer in our stories, since that's some primary documentation for folks. With this PR, we can cut a release and then begin updating consumer repos accordingly. First, to include this update, then to migrate them off the old ways and onto the new. ### Release process: Once this small stack of PRs are landed and released, the following PRs need to be updated with that release, then landed/deployed: - Perseus: Khan/perseus#2007 - Webapp - Khan/webapp#28105 - Khan/webapp#28127 Issue: WB-1812 ## Test plan: `yarn test` `yarn typecheck` `yarn start:storybook` Author: somewhatabstract Reviewers: jandrade, somewhatabstract Required Reviewers: Approved By: jandrade Checks: ⌛ Lint / Lint (ubuntu-latest, 20.x), ⌛ Check build sizes (ubuntu-latest, 20.x), ⌛ Chromatic - Build on regular PRs / chromatic (ubuntu-latest, 20.x), ⌛ Publish npm snapshot (ubuntu-latest, 20.x), ⌛ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ⏭️ Chromatic - Skip on Release PR (changesets), ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), ✅ gerald, ⏭️ dependabot Pull Request URL: #2391
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
## Summary: The `changesets` tooling supports a configuration option that causes `yarn changeset` to automatically add and commit the created file ([docs](https://github.com/changesets/changesets/blob/main/docs/config-file-options.md#commit-boolean-or-module-path-as-a-string-or-a-tuple-like-modulepath-string-options-any)). NOTE: If there are existing changes in your local repo, these changes _will not_ be included in the commit (so it's safe to run `yarn changeset` no matter the state of your repo). There are PRs in other Khan Academy repos that use the Changesets tooling. We'll only land this if there's consensus across these other repos too: * Khan/wonder-stuff#1097 * #2394 (this PR) * Khan/perseus#2008 Issue: "none" ## Test plan: `yarn changeset` Fill in the details Once complete, run `git log` and you will see that the changeset entry you created has been added and committed to git - all that's left is to create the PR! Author: jeremywiebe Reviewers: jandrade, somewhatabstract Required Reviewers: Approved By: jandrade Checks: ✅ Chromatic - Get results on regular PRs (ubuntu-latest, 20.x), ✅ Test / Test (ubuntu-latest, 20.x, 2/2), ✅ Test / Test (ubuntu-latest, 20.x, 1/2), ✅ Lint / Lint (ubuntu-latest, 20.x), ✅ Check build sizes (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Chromatic - Build on regular PRs / chromatic (ubuntu-latest, 20.x), ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), ⏭️ Chromatic - Skip on Release PR (changesets), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ gerald, ✅ Test / Test (ubuntu-latest, 20.x, 2/2), ✅ Test / Test (ubuntu-latest, 20.x, 1/2), ✅ Lint / Lint (ubuntu-latest, 20.x), ✅ Check build sizes (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), ✅ gerald, ✅ Chromatic - Get results on regular PRs (ubuntu-latest, 20.x), ✅ Test / Test (ubuntu-latest, 20.x, 2/2), ✅ Test / Test (ubuntu-latest, 20.x, 1/2), ✅ Lint / Lint (ubuntu-latest, 20.x), ✅ Check build sizes (ubuntu-latest, 20.x), ✅ Chromatic - Build on regular PRs / chromatic (ubuntu-latest, 20.x), ⏭️ Publish npm snapshot, ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ⏭️ Chromatic - Skip on Release PR (changesets), ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), ✅ gerald, ⌛ undefined Pull Request URL: #2394
## Summary: This deletes the components and hooks that we had for generating hydration-safe identifiers. We are now using the built-in `useId` hook internally, and expect our consumers to do the same (or the `Id` component we provide for more complex cases). This should not be released until the following PRs are merged and deployed to webapp: - Khan/webapp#28161 - Khan/webapp#28156 Issue: WB-1812 ## Test plan: `yarn test` `yarn typecheck` Author: somewhatabstract Reviewers: beaesguerra Required Reviewers: Approved By: beaesguerra Checks: ✅ Chromatic - Get results on regular PRs (ubuntu-latest, 20.x), ✅ Lint / Lint (ubuntu-latest, 20.x), ✅ Test / Test (ubuntu-latest, 20.x, 2/2), ✅ Test / Test (ubuntu-latest, 20.x, 1/2), ✅ Check build sizes (ubuntu-latest, 20.x), ✅ Chromatic - Build on regular PRs / chromatic (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ⏭️ Chromatic - Skip on Release PR (changesets), ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ gerald, ✅ Chromatic - Get results on regular PRs (ubuntu-latest, 20.x), ✅ Lint / Lint (ubuntu-latest, 20.x), ✅ Test / Test (ubuntu-latest, 20.x, 2/2), ✅ Test / Test (ubuntu-latest, 20.x, 1/2), ✅ Check build sizes (ubuntu-latest, 20.x), ✅ Chromatic - Build on regular PRs / chromatic (ubuntu-latest, 20.x), ⏭️ Chromatic - Skip on Release PR (changesets), ⏹️ [cancelled] Chromatic - Get results on regular PRs, ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), ✅ gerald, ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Lint / Lint (ubuntu-latest, 20.x), ✅ Test / Test (ubuntu-latest, 20.x, 2/2), ✅ Test / Test (ubuntu-latest, 20.x, 1/2), ✅ Check build sizes (ubuntu-latest, 20.x), ⏹️ [cancelled] Chromatic - Build on regular PRs / chromatic (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ⏭️ Chromatic - Skip on Release PR (changesets), ⌛ undefined Pull Request URL: #2398
Releases: @khanacademy/[email protected] @khanacademy/[email protected] @khanacademy/[email protected] @khanacademy/[email protected] @khanacademy/[email protected] @khanacademy/[email protected] @khanacademy/[email protected] @khanacademy/[email protected] @khanacademy/[email protected] @khanacademy/[email protected] @khanacademy/[email protected] @khanacademy/[email protected] @khanacademy/[email protected] @khanacademy/[email protected] @khanacademy/[email protected] @khanacademy/[email protected] @khanacademy/[email protected] @khanacademy/[email protected] @khanacademy/[email protected] @khanacademy/[email protected] @khanacademy/[email protected] @khanacademy/[email protected] @khanacademy/[email protected] @khanacademy/[email protected] @khanacademy/[email protected] @khanacademy/[email protected] @khanacademy/[email protected] [skip ci] Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
…ace (#2405) ## Summary: Adds `width: 100%` to `legend` element to allow expanding it to fill the available space. Note that this applies to both `RadioGroup` and `CheckboxGroup`. Issue: XXX-XXXX ## Test plan: Navigate to `/?path=/docs/packages-form-radiogroup--docs#custom%20label`. Verify that the legend element is now full-bleed. Author: jandrade Reviewers: beaesguerra Required Reviewers: Approved By: beaesguerra Checks: ✅ Chromatic - Get results on regular PRs (ubuntu-latest, 20.x), ✅ Test / Test (ubuntu-latest, 20.x, 2/2), ✅ Test / Test (ubuntu-latest, 20.x, 1/2), ✅ Lint / Lint (ubuntu-latest, 20.x), ✅ Check build sizes (ubuntu-latest, 20.x), ✅ Chromatic - Build on regular PRs / chromatic (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ⏭️ Chromatic - Skip on Release PR (changesets), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ gerald, ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), ⏭️ dependabot Pull Request URL: #2405
* RELEASING: Releasing 4 package(s) Releases: @khanacademy/[email protected] @khanacademy/[email protected] @khanacademy/[email protected] @khanacademy/[email protected] [skip ci] * Trigger empty commit --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Juan Andrade <[email protected]>
…urbosnap (#2408) ## Summary: While investigating the performance of Chromatic/Turbosnap, I found that chromatic was disabling Turbosnap of the storybook stories in a lot of builds because apparently we were making changing changes in files inside the `.storybook` folder. This is not the case as those builds didn't include changes in such files, but this made me realize that we could optimize the performance of Turbosnap by moving the reusable components from the `.storybook` folder to the `__docs__` folder. Issue: XXX-XXXX ## Test plan: Verify that Storybook still works as expected and check the Chromatic build to see if Turbosnap is enabled for the stories that were previously disabled. Author: jandrade Reviewers: beaesguerra Required Reviewers: Approved By: beaesguerra Checks: ✅ Chromatic - Get results on regular PRs (ubuntu-latest, 20.x), ✅ Test / Test (ubuntu-latest, 20.x, 2/2), ✅ Lint / Lint (ubuntu-latest, 20.x), ✅ Test / Test (ubuntu-latest, 20.x, 1/2), ✅ Check build sizes (ubuntu-latest, 20.x), ✅ Chromatic - Build on regular PRs / chromatic (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ⏭️ Chromatic - Skip on Release PR (changesets), ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), ✅ gerald, ⏭️ dependabot, ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x) Pull Request URL: #2408
…2409) ## Summary: I recently updated our Changesets config to auto-commit the `.md` file when using `yarn changset` to describe a set of changes. This broke our releases as the default behaviour is that Changesets adds `[skip ci]` to the commit created when running `yarn changesets version`. No CI on that PR/commit, meant that none of our release Github Actions ran! So this PR fixes the config so that Changesets no longer adds `[skip ci]` to any commits, but still auto-commits them. Issue: "none" ## Test plan: Land this PR Merge `main` into an open Version Packages PR Land that PR and ensure the release goes out Author: jeremywiebe Reviewers: jandrade Required Reviewers: Approved By: jandrade Checks: ✅ Chromatic - Get results on regular PRs (ubuntu-latest, 20.x), ✅ Test / Test (ubuntu-latest, 20.x, 2/2), ✅ Lint / Lint (ubuntu-latest, 20.x), ✅ Test / Test (ubuntu-latest, 20.x, 1/2), ✅ Check build sizes (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), ✅ gerald, ✅ Lint / Lint (ubuntu-latest, 20.x), ✅ Test / Test (ubuntu-latest, 20.x, 2/2), ✅ Test / Test (ubuntu-latest, 20.x, 1/2), ✅ Check build sizes (ubuntu-latest, 20.x), ✅ Chromatic - Build on regular PRs / chromatic (ubuntu-latest, 20.x), ⏭️ Publish npm snapshot, ⏭️ Chromatic - Skip on Release PR (changesets), ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), ⏭️ dependabot, ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ gerald Pull Request URL: #2409
## Summary: This PR is meant to bump the version of the form package to test changesets. We are seeing if a change being applied in #2409 will fix the issue we are seeing with changesets not being triggering PR checks at all. This is causing that packages are not being published to NPM. Issue: XXX-XXXX ## Test plan: Land this PR and see if the changesets checks are triggered once the Release PR is created. Author: jandrade Reviewers: beaesguerra, jeremywiebe Required Reviewers: Approved By: beaesguerra Checks: ✅ Chromatic - Get results on regular PRs (ubuntu-latest, 20.x), ✅ Test / Test (ubuntu-latest, 20.x, 2/2), ✅ Test / Test (ubuntu-latest, 20.x, 1/2), ✅ Lint / Lint (ubuntu-latest, 20.x), ✅ Check build sizes (ubuntu-latest, 20.x), ✅ Chromatic - Build on regular PRs / chromatic (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ⏭️ Chromatic - Skip on Release PR (changesets), ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ gerald, ✅ Chromatic - Get results on regular PRs (ubuntu-latest, 20.x), ✅ Test / Test (ubuntu-latest, 20.x, 2/2), ✅ Test / Test (ubuntu-latest, 20.x, 1/2), ✅ Lint / Lint (ubuntu-latest, 20.x), ✅ Check build sizes (ubuntu-latest, 20.x), ✅ Chromatic - Build on regular PRs / chromatic (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ⏭️ Chromatic - Skip on Release PR (changesets), ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), ✅ gerald, ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ⏭️ dependabot Pull Request URL: #2410
* RELEASING: Releasing 4 package(s) Releases: @khanacademy/[email protected] @khanacademy/[email protected] @khanacademy/[email protected] @khanacademy/[email protected] * Trigger empty commit --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Juan Andrade <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
Experimenting with the new announcer component to see if it can be used to
announce all the necessary information for the combobox.
Issue: https://khanacademy.atlassian.net/browse/WB-1780
Test plan:
Enable Voice Over.
Navigate to the Combobox docs and interact with the combobox to see if the
announcer announces the necessary information.