Skip to content
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

[Modal] Fix activator not gaining focus when modal is closed #11266

Merged
merged 2 commits into from
Dec 5, 2023

Conversation

LauraAubin
Copy link
Contributor

@LauraAubin LauraAubin commented Nov 30, 2023

WHY are these changes introduced?

Fixes the modal activator not regaining focus when the modal is closed.

Will also resolve #10493.

WHAT is this pull request doing?

Previously in this PR, the Box was removed in favour of using cloneElement. This was done in order to obtain inline styling since the Box is a div by default. However, cloneElement was also passing the ref as a prop, which assumes that the activator you are passing also accepts ref, which in most cases is not the case (i.e, Button). As a result, the ref to the activator was always null.

This PR reverts this change and reimplements the Box so that the ref can be maintained. This PR also introduces a new prop activatorWrapper, which enables the consumer to optionally convert the Box into whatever element they choose. In the case of inline styling, we can now set the activator to be a span. This new prop activatorWrapper is a consistent pattern across this repo, i.e <Tooltip />.

How to 🎩

Since many modals across the admin are either passing a ref, or simply do not have the activator hooked up, I added a simple example to the top of the orders list page in the admin with an element activator.

Screen.Recording.2023-12-01.at.2.08.29.PM.mov
  • Orders list: With these changes
  • Open the modal by clicking on the button Open test modal.
  • Press tab. Once on the exit button, press enter to close the modal.
  • Ensure focus is redirected back to the Open test modal button.
  • Ensure that the following console error does not exist:
Warning: Function components cannot be given refs. Attempts to access this ref will fail. Did you mean to use React.forwardRef()

Here is the same example implemented on a separate Spinstance without these changes:


🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

🎩 checklist

// Causes a circular dependency that causes the whole test file to be unrunnable
// eslint-disable-next-line jest/no-disabled-tests
it.skip('focuses the activator when the activator is an element on close', () => {
it('wraps the activator in a div by default', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests can now be un-skipped and have been fixed!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's unfortunate these tests were skipped (and I didn't notice) at the time I shipped my PR. It would have caught that!

@LauraAubin
Copy link
Contributor Author

/snapit

@LauraAubin
Copy link
Contributor Author

LauraAubin commented Nov 30, 2023

cc @joelzwarrington, we'll need to coordinate and add the activatorWrapper="span" prop to your area once this Polaris version is released and updated in Web. I'm happy to make that change, just keeping you in the loop!

Copy link
Contributor

🫰✨ Thanks @LauraAubin! Your snapshots have been published to npm.

Test the snapshots by updating your package.json with the newly published versions:

yarn add @shopify/[email protected]
yarn add @shopify/[email protected]
yarn add @shopify/[email protected]
yarn add @shopify/[email protected]
yarn add @shopify/[email protected]

@LauraAubin LauraAubin force-pushed the fix-modal-activator-focus-on-close branch from db8b3fb to d29c848 Compare November 30, 2023 22:57
@Shopify Shopify deleted a comment from github-actions bot Nov 30, 2023
@LauraAubin LauraAubin force-pushed the fix-modal-activator-focus-on-close branch from 2d1f73a to d29c848 Compare December 1, 2023 18:55
@Shopify Shopify deleted a comment from github-actions bot Dec 1, 2023
@Shopify Shopify deleted a comment from github-actions bot Dec 1, 2023
@LauraAubin LauraAubin marked this pull request as ready for review December 1, 2023 19:12
@LauraAubin LauraAubin requested a review from a team December 1, 2023 19:13
Copy link
Member

@chloerice chloerice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing this Laura!!

.changeset/empty-poets-wait.md Outdated Show resolved Hide resolved
@LauraAubin LauraAubin merged commit bf0593e into main Dec 5, 2023
9 checks passed
@LauraAubin LauraAubin deleted the fix-modal-activator-focus-on-close branch December 5, 2023 19:57
sophschneider added a commit that referenced this pull request Dec 6, 2023
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to main, this PR will
be updated.


# Releases
## @shopify/[email protected]

### Minor Changes

- [#11073](#11073)
[`c3cad73cb`](c3cad73)
Thanks [@lntn2022](https://github.com/lntn2022)! - Add PayoutsBlocked
icon


- [#11136](#11136)
[`0b1961c16`](0b1961c)
Thanks [@alex-page](https://github.com/alex-page)! - Update Metaobject
and MetaobjectReference icon SVGs

## @shopify/[email protected]

### Minor Changes

- [#11166](#11166)
[`456f3da42`](456f3da)
Thanks [@sophschneider](https://github.com/sophschneider)! - Added Card
component default value for roundedAbove prop


- [#11211](#11211)
[`07aa5e979`](07aa5e9)
Thanks [@sophschneider](https://github.com/sophschneider)! - Added
`size` prop with `slim` and `medium` options to `TextField`


- [#11266](#11266)
[`bf0593e20`](bf0593e)
Thanks [@LauraAubin](https://github.com/LauraAubin)! - - Fixed `Modal`
`activator` not regaining focus on close
- Added an `activatorWrapper` prop to `Modal` to support setting the
element that wraps the `activator`


- [#11201](#11201)
[`eca971dca`](eca971d)
Thanks [@laurkim](https://github.com/laurkim)! - Added documentation for
`Card` examples to support `LegacyCard` compositions


- [#11261](#11261)
[`32cfbecb1`](32cfbec)
Thanks [@lgriffee](https://github.com/lgriffee)! - Updated `Avatar`
background and text colors


- [#11219](#11219)
[`97683ac05`](97683ac)
Thanks [@matallo](https://github.com/matallo)! - - Bumped
`color-text-magic-secondary` to purple 13
    -   Added `tone` prop with `magic` value to `Select`
    -   Added `magic` value to `tone` prop of `Text`
    -   Added `magic-subdued` value to `tone` prop of `Text`


- [#11264](#11264)
[`773daaa0f`](773daaa)
Thanks [@ryanschingeck](https://github.com/ryanschingeck)! - Added
`maxWidth` prop to SkeletonDisplayText component


- [#11172](#11172)
[`64ee75039`](64ee750)
Thanks [@yurm04](https://github.com/yurm04)! - Added the `key` prop to
`Select` component `StrictOption`


- [#11170](#11170)
[`79cadcf4f`](79cadcf)
Thanks [@laurkim](https://github.com/laurkim)! - Added support for
`paddingInline` and `paddingBlock` on `Box` component with updated
documentation


- [#11115](#11115)
[`45deb1941`](45deb19)
Thanks [@fatimasajadi](https://github.com/fatimasajadi)! - Fixed hover
state of `IndexTable.Row` when `selectable` is `false`


- [#10633](#10633)
[`53fe61479`](53fe614)
Thanks [@mattkubej](https://github.com/mattkubej)! - Updated IndexTable,
ResourceList, and DataTable to have built-in pagination props


- [#10726](#10726)
[`35d92bcd8`](35d92bc)
Thanks [@mrcthms](https://github.com/mrcthms)! - Updated Filters to not
render or perform filters logic if the filters array is empty


- [#10800](#10800)
[`9159e5083`](9159e50)
Thanks [@sirgalleto](https://github.com/sirgalleto)! - Added support for
an `EditColumns` button rendered in the `IndexFilters` deprecating the
`Tabs`'s `edit-columns` action.

    -   `IndexFilters`
- Added support for rendering an Edit Columns button using the
`showEditColumnsButton` flag.
- Added the edition `mode` to the `onEditStart(mode)` callback.
    -   `Tabs`
        -   Removed the `edit-columns` action type.


- [#11221](#11221)
[`799276156`](7992761)
Thanks [@mrcthms](https://github.com/mrcthms)! - Updated Pagination to
be correct height of 40px when in the table variant


- [#11263](#11263)
[`04b8fb370`](04b8fb3)
Thanks [@alex-page](https://github.com/alex-page)! - Add storybook
example for all icons in @shopify/polaris-icons


- [#11094](#11094)
[`2c5ca9900`](2c5ca99)
Thanks [@laurkim](https://github.com/laurkim)! - Added support for
`bodyXs` variant and fixed font weight for `headingLg` variant in `Text`
component.
Updated references to font tokens from Polaris v11 to v12 in `Text`
component documentation


- [#11036](#11036)
[`1459f773d`](1459f77)
Thanks [@hughnguy](https://github.com/hughnguy)! - Fixed `TextField`
events not bubbling up when `multiline`

### Patch Changes

- [#10987](#10987)
[`00374f85a`](00374f8)
Thanks [@kyledurand](https://github.com/kyledurand)! - Simplified
`Button` code


- [#11211](#11211)
[`07aa5e979`](07aa5e9)
Thanks [@sophschneider](https://github.com/sophschneider)! - Fixed
transparent background and wrapping in `IndexFilters` on small screens


- [#11211](#11211)
[`07aa5e979`](07aa5e9)
Thanks [@sophschneider](https://github.com/sophschneider)! - Added
`SearchMinor` icon to `Filters` search field when `mdUp`


- [#11101](#11101)
[`6297e524a`](6297e52)
Thanks [@AndrewMusgrave](https://github.com/AndrewMusgrave)! - Fixed ref
error in `Tabs` for `CreateViewModal` and removed promoted bulk action
warnings


- [#11203](#11203)
[`8b9ded242`](8b9ded2)
Thanks [@kyledurand](https://github.com/kyledurand)! - Updated
IndexTable documentation for when to hide bulk actions


- [#11270](#11270)
[`09df04ca5`](09df04c)
Thanks [@moraleslevi](https://github.com/moraleslevi)! - Remove
scrollbars from TextField vertical content


- [#11245](#11245)
[`165c860c2`](165c860)
Thanks [@matallo](https://github.com/matallo)! - - Fixed `onFocus` and
`onBlur` events of Select component


- [#11237](#11237)
[`6b6e27ce0`](6b6e27c)
Thanks [@alex-page](https://github.com/alex-page)! - Remove unused
custom icon from dropzone that was used in a test


- [#11103](#11103)
[`786ee94b4`](786ee94)
Thanks [@kyledurand](https://github.com/kyledurand)! - Fixed Filters
SearchField background in dark mode


- [#11211](#11211)
[`07aa5e979`](07aa5e9)
Thanks [@sophschneider](https://github.com/sophschneider)! - Fixed
`TextField` clear button to not render when hidden


- [#11169](#11169)
[`90de38843`](90de388)
Thanks [@kyledurand](https://github.com/kyledurand)! - Added expanded
styling to Button


- [#11206](#11206)
[`0e8ab42b4`](0e8ab42)
Thanks [@sophschneider](https://github.com/sophschneider)! - Fixed
disjointed Navigation arrow on small screens


- [#10804](#10804)
[`fe8491507`](fe84915)
Thanks [@sam-b-rose](https://github.com/sam-b-rose)! - Add support for
using breakpoint tokens in CSS by using `@custom-media`


- [#11124](#11124)
[`ad504d5be`](ad504d5)
Thanks [@sarahill](https://github.com/sarahill)! - Removed bevel from
`pressed` `Button` when focused


- [#11281](#11281)
[`b0edfbb92`](b0edfbb)
Thanks [@sirgalleto](https://github.com/sirgalleto)! - Restores the
Tab's `edit-columns` action type


- [#11238](#11238)
[`2df27ed0b`](2df27ed)
Thanks [@alex-page](https://github.com/alex-page)! - Conditionally
render the accessibilityLabel when it is provided


- [#11168](#11168)
[`9c3dd913c`](9c3dd91)
Thanks [@mattkubej](https://github.com/mattkubej)! - [Page] prevent
vertical content shift of header with metadata and actions


- [#11211](#11211)
[`07aa5e979`](07aa5e9)
Thanks [@sophschneider](https://github.com/sophschneider)! - Replaced
custom `Filters` input with Polaris `TextField`


- [#11123](#11123)
[`ac45afda8`](ac45afd)
Thanks [@sarahill](https://github.com/sarahill)! - Updated `Button` base
state colors to use `fill` tokens


- [#10599](#10599)
[`b7219863d`](b721986)
Thanks [@oksanashopify](https://github.com/oksanashopify)! - Added child
type to IndexTable row


- [#11105](#11105)
[`ecbd6c137`](ecbd6c1)
Thanks [@sarahill](https://github.com/sarahill)! - Fixed `Tag` icon and
disabled state colors


- [#11012](#11012)
[`c25478fba`](c25478f)
Thanks [@chloerice](https://github.com/chloerice)! - Fixed `FormLayout`
spacing


- [#10753](#10753)
[`c849ff468`](c849ff4)
Thanks [@stephxshopify](https://github.com/stephxshopify)! - [Modal]
Disallow vertical scroll with use of noScroll


- Updated dependencies
\[[`c58632afa`](c58632a),
[`c3cad73cb`](c3cad73),
[`0b1961c16`](0b1961c),
[`32cfbecb1`](32cfbec),
[`97683ac05`](97683ac),
[`fe8491507`](fe84915)]:
    -   @shopify/[email protected]
    -   @shopify/[email protected]

## @shopify/[email protected]

### Minor Changes

- [#11173](#11173)
[`c58632afa`](c58632a)
Thanks [@jesstelford](https://github.com/jesstelford)! - Add 0 tokens
where missing as per [the team's
decision](https://github.com/Shopify/polaris/discussions/7334#discussioncomment-4988991).


- [#11261](#11261)
[`32cfbecb1`](32cfbec)
Thanks [@lgriffee](https://github.com/lgriffee)! - Updated `Avatar`
background and text colors


- [#11219](#11219)
[`97683ac05`](97683ac)
Thanks [@matallo](https://github.com/matallo)! - - Bumped
`color-text-magic-secondary` to purple 13
    -   Added `tone` prop with `magic` value to `Select`
    -   Added `magic` value to `tone` prop of `Text`
    -   Added `magic-subdued` value to `tone` prop of `Text`


- [#10804](#10804)
[`fe8491507`](fe84915)
Thanks [@sam-b-rose](https://github.com/sam-b-rose)! - Add support for
using breakpoint tokens in CSS by using `@custom-media`

## @shopify/[email protected]

### Patch Changes

- Updated dependencies
\[[`c58632afa`](c58632a),
[`32cfbecb1`](32cfbec),
[`97683ac05`](97683ac),
[`fe8491507`](fe84915)]:
    -   @shopify/[email protected]
    -   @shopify/[email protected]

## @shopify/[email protected]

### Patch Changes

- Updated dependencies
\[[`c58632afa`](c58632a),
[`32cfbecb1`](32cfbec),
[`97683ac05`](97683ac),
[`fe8491507`](fe84915)]:
    -   @shopify/[email protected]

## [email protected]

### Minor Changes

- [#11122](#11122)
[`636d133e2`](636d133)
Thanks [@alex-page](https://github.com/alex-page)! - Update dependencies
and published files to fix vsce publish

### Patch Changes

- Updated dependencies
\[[`c58632afa`](c58632a),
[`32cfbecb1`](32cfbec),
[`97683ac05`](97683ac),
[`fe8491507`](fe84915)]:
    -   @shopify/[email protected]

## [email protected]

### Minor Changes

- [#11170](#11170)
[`79cadcf4f`](79cadcf)
Thanks [@laurkim](https://github.com/laurkim)! - Added support for
`paddingInline` and `paddingBlock` on `Box` component with updated
documentation


- [#11109](#11109)
[`533543abd`](533543a)
Thanks [@heyjoethomas](https://github.com/heyjoethomas)! - Removed
duplicate content on icon contribution


- [#11078](#11078)
[`ed5d7d6bf`](ed5d7d6)
Thanks [@heyjoethomas](https://github.com/heyjoethomas)! - Update icon
contribution guidance


- [#11104](#11104)
[`0e3f591ad`](0e3f591)
Thanks [@heyjoethomas](https://github.com/heyjoethomas)! - Added new
content section on how to add an issue to our board rather than creating
your own pr.

### Patch Changes

- [#11203](#11203)
[`8b9ded242`](8b9ded2)
Thanks [@kyledurand](https://github.com/kyledurand)! - Updated
IndexTable documentation for when to hide bulk actions


- [#11198](#11198)
[`351433f59`](351433f)
Thanks [@kyledurand](https://github.com/kyledurand)! - Fixed typos in
naming docs


- [#11131](#11131)
[`9db9731c7`](9db9731)
Thanks [@sam-b-rose](https://github.com/sam-b-rose)! - Fixed icon search
and query params from URL


- [#11218](#11218)
[`b83ed4edb`](b83ed4e)
Thanks [@sam-b-rose](https://github.com/sam-b-rose)! - Fixed link to
tokens from global search


- [#11260](#11260)
[`61cacbfbb`](61cacbf)
Thanks [@LauraAubin](https://github.com/LauraAubin)! - Fix column
content alignment for the IndexTable and IndexFilters total column


- [#11094](#11094)
[`2c5ca9900`](2c5ca99)
Thanks [@laurkim](https://github.com/laurkim)! - Added support for
`bodyXs` variant and fixed font weight for `headingLg` variant in `Text`
component.
Updated references to font tokens from Polaris v11 to v12 in `Text`
component documentation
- Updated dependencies
\[[`00374f85a`](00374f8),
[`07aa5e979`](07aa5e9),
[`07aa5e979`](07aa5e9),
[`456f3da42`](456f3da),
[`6297e524a`](6297e52),
[`c58632afa`](c58632a),
[`8b9ded242`](8b9ded2),
[`07aa5e979`](07aa5e9),
[`bf0593e20`](bf0593e),
[`eca971dca`](eca971d),
[`09df04ca5`](09df04c),
[`165c860c2`](165c860),
[`c3cad73cb`](c3cad73),
[`6b6e27ce0`](6b6e27c),
[`0b1961c16`](0b1961c),
[`786ee94b4`](786ee94),
[`32cfbecb1`](32cfbec),
[`07aa5e979`](07aa5e9),
[`90de38843`](90de388),
[`97683ac05`](97683ac),
[`773daaa0f`](773daaa),
[`0e8ab42b4`](0e8ab42),
[`fe8491507`](fe84915),
[`ad504d5be`](ad504d5),
[`b0edfbb92`](b0edfbb),
[`64ee75039`](64ee750),
[`79cadcf4f`](79cadcf),
[`45deb1941`](45deb19),
[`53fe61479`](53fe614),
[`35d92bcd8`](35d92bc),
[`2df27ed0b`](2df27ed),
[`9c3dd913c`](9c3dd91),
[`07aa5e979`](07aa5e9),
[`9159e5083`](9159e50),
[`799276156`](7992761),
[`ac45afda8`](ac45afd),
[`b7219863d`](b721986),
[`ecbd6c137`](ecbd6c1),
[`c25478fba`](c25478f),
[`04b8fb370`](04b8fb3),
[`c849ff468`](c849ff4),
[`2c5ca9900`](2c5ca99),
[`1459f773d`](1459f77)]:
    -   @shopify/[email protected]
    -   @shopify/[email protected]
    -   @shopify/[email protected]

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Sophie Schneider <[email protected]>
AnnaCheba pushed a commit to AnnaCheba/polaris that referenced this pull request Apr 22, 2024
…#11266)

Fix modal activator focus on close and add new optional activatorWrapper prop
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Modal now generates React warning using a function component as activator
3 participants