-
Notifications
You must be signed in to change notification settings - Fork 255
fix: allow reassignment of focus and blur methods on `HTMLElement…
#1265
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
fix: allow reassignment of focus and blur methods on `HTMLElement…
#1265
Conversation
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
|
I ran into the same issue when trying to upgrade |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for contributing to this library.
Replacing the accessor descriptor with a data descriptor removes the extra logic.
IMHO this also makes the test redundant.
Supporting the reassignment would also require us to expose proper setup and teardown, as the linked teardown function in Chakra-UI would break our implementation if their setup function runs before our setup.
But this is out of scope here – I will tackle this in a separate PR.
|
Thanks for the comments, @ph-fritsche. I made the requested changes and removed the test. |
|
@ph-fritsche can this fix get released soon? |
#1270 should fix that but I didn't have time to check yet |
Just for posterity, I want to mention that the Jasmine |
|
Hey there, thanks for solving the issue! Any idea when there will be a new release? |
|
Hi @ph-fritsche , could this fix be released soon please? Thank you |
|
Could we release this soon please?! 🙏 |
|
Would be much appreciated if this change can release soon 🙏 This fixes an issue when testing code that uses react-aria: #1277 The app I am working on uses react-aria via headless-ui. |
|
Hi all, any update on a release? This is blocking v14 upgrades :/ |
|
It’s been over four months since the maintainer had meaningful activity. Perhaps this repo has been effectively abandoned. (I mean nothing snide; could be a medical emergency etc) |
Newer versions have an issue preventing reassignment of `HTMLElement.prototype.focus`, which is fixed in source, but not published in an available release. The reassignment is part of Ariakit's testing polyfills. See: - testing-library/user-event#1265 - https://github.com/ariakit/ariakit/blob/016f8a3ee9d3e59c593e2971221111a7fa137727/packages/ariakit-test/src/__utils.ts#L34
* Update Storybook to v10 with Vite builder * Update TypeScript base config to use bundler module resolution * Update storybook to v10: Misc updates (#74491) * Use preset for addon * Convert main.mjs to main.ts * Disable default links for v10 * Update Storybook test-runner to v0.24.2 * Remove webpack from ignored licenses list Co-authored-by: manzoorwanijk <[email protected]> Co-authored-by: aduth <[email protected]> * Avoid variable shadowing by type-checking as satisfies * Align types export on jest-console package * Update E2E Storybook configuration for v10 * Fix file extension on README Storybook configuration file reference Co-Authored-By: Manzoor Wani <[email protected]> * Use `?inline` instead of `?raw` for importing compiled Sass `?raw` imports raw content of the referenced file. `?inline` assigns "the processed CSS string" See: https://vite.dev/guide/features#disabling-css-injection-into-the-page * Try renaming JavaScript Story to TypeScript * Pin user-event to 14.4.3 Newer versions have an issue preventing reassignment of `HTMLElement.prototype.focus`, which is fixed in source, but not published in an available release. The reassignment is part of Ariakit's testing polyfills. See: - testing-library/user-event#1265 - https://github.com/ariakit/ariakit/blob/016f8a3ee9d3e59c593e2971221111a7fa137727/packages/ariakit-test/src/__utils.ts#L34 * Sync Storybook dependencies at common version Also removes stale and conflicting entries in package-lock.json * Update reference to storybook/react-webpack5 dependency * Fix Storybook build by updating README import in introduction files Update the import statements in introduction.mdx files to use '?raw' when importing README.md. * Remove moduleResolution from sync tsconfig * Add missing dependency array to useEffect * Remove unnecessary .lazy suffix from scss files --------- Co-authored-by: Manzoor Wani <[email protected]> Co-authored-by: manzoorwanijk <[email protected]> Co-authored-by: Manzoor Wani <[email protected]> Co-authored-by: aduth <[email protected]> Co-authored-by: manzoorwanijk <[email protected]> Co-authored-by: jsnajdr <[email protected]> Co-authored-by: mirka <[email protected]>
* Update Storybook to v10 with Vite builder * Update TypeScript base config to use bundler module resolution * Update storybook to v10: Misc updates (#74491) * Use preset for addon * Convert main.mjs to main.ts * Disable default links for v10 * Update Storybook test-runner to v0.24.2 * Remove webpack from ignored licenses list Co-authored-by: manzoorwanijk <[email protected]> Co-authored-by: aduth <[email protected]> * Avoid variable shadowing by type-checking as satisfies * Align types export on jest-console package * Update E2E Storybook configuration for v10 * Fix file extension on README Storybook configuration file reference Co-Authored-By: Manzoor Wani <[email protected]> * Use `?inline` instead of `?raw` for importing compiled Sass `?raw` imports raw content of the referenced file. `?inline` assigns "the processed CSS string" See: https://vite.dev/guide/features#disabling-css-injection-into-the-page * Try renaming JavaScript Story to TypeScript * Pin user-event to 14.4.3 Newer versions have an issue preventing reassignment of `HTMLElement.prototype.focus`, which is fixed in source, but not published in an available release. The reassignment is part of Ariakit's testing polyfills. See: - testing-library/user-event#1265 - https://github.com/ariakit/ariakit/blob/016f8a3ee9d3e59c593e2971221111a7fa137727/packages/ariakit-test/src/__utils.ts#L34 * Sync Storybook dependencies at common version Also removes stale and conflicting entries in package-lock.json * Update reference to storybook/react-webpack5 dependency * Fix Storybook build by updating README import in introduction files Update the import statements in introduction.mdx files to use '?raw' when importing README.md. * Remove moduleResolution from sync tsconfig * Add missing dependency array to useEffect * Remove unnecessary .lazy suffix from scss files --------- Co-authored-by: Manzoor Wani <[email protected]> Co-authored-by: manzoorwanijk <[email protected]> Co-authored-by: Manzoor Wani <[email protected]> Co-authored-by: aduth <[email protected]> Co-authored-by: manzoorwanijk <[email protected]> Co-authored-by: jsnajdr <[email protected]> Co-authored-by: mirka <[email protected]> Source: WordPress/gutenberg@20d9161
What
Reinstates the ability to reassign
HTMLElement.prototype.focusandHTMLElement.prototype.blurthat was removed in #1252.Why
Some UI libraries (Chakra UI, for example) override the
focusand/orblurmethods on theHTMLElementprototype. #1252 introduced a patchedfocusimplementation that only included a getter, which means these libraries error when run with@testing-library/user-eventbecause there is no setter.How
This adds a setter to the patched
focusandblurmethods.Checklist
I'm not super confident that this is the correct or preferred way to resolve the issue, but it works for my use cases.
This line from
@zag-js/radio-group(a dependency of@chakra-ui/react) was the operative code in my case: https://github.com/chakra-ui/zag/blob/c1ffbfe1eb09028c837b305942f34bb77b9ac4fc/packages/utilities/focus-visible/src/index.ts#L143