[Filters] Fix vanishing applied filters bug #10429

mrcthms · 2023-09-13T14:11:41Z

WHY are these changes introduced?

Fixes an issue where if you have an applied filter set from the props when the component initiates, and then you remove the selected filter option, the filter pill disappears from the view. The correct behaviour is that the filter pill should remain, but be empty.

Video of existing issue

267128879-d255bd8c-8cfc-4a0c-a02f-055d04eca5ce.mov

Video showing new intended behaviour

Screen.Recording.2023-09-13.at.15.06.56.mov

WHAT is this pull request doing?

The issue stemmed from having two sources of local pinned filters, and them behaving in different ways. Now, by only using the localPinnedFilters value, and setting the initial state value on component mount, we can expect the component to behave identically for both filter pills that are rendered on mount, and filter pills that are rendered as a result of a state change within the component.

How to 🎩

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

🎩 checklist

Tested on mobile
Tested on multiple browsers
Tested for accessibility
Updated the component's README.md with documentation changes
Tophatted documentation changes in the style guide

mrcthms · 2023-09-13T14:11:52Z

/snapit

github-actions · 2023-09-13T14:16:17Z

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

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

yarn add @shopify/polaris-migrator@0.0.0-snapshot-release-20230913141409

yarn add @shopify/polaris@0.0.0-snapshot-release-20230913141409

yarn add @shopify/polaris-tokens@0.0.0-snapshot-release-20230913141409

yarn add @shopify/stylelint-polaris@0.0.0-snapshot-release-20230913141409

.changeset/beige-jeans-scream.md

Co-authored-by: Chloe Rice <chloerice@users.noreply.github.com>

@mrcthms

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/polaris@11.18.0 ### Minor Changes - [#10429](#10429) [`2e65a7e98`](2e65a7e) Thanks [@mrcthms](https://github.com/mrcthms)! - Fixed `Filters` pinned filter pill not remaining when applied values are cleared - [#10394](#10394) [`b7f25f07f`](b7f25f0) Thanks [@mrcthms](https://github.com/mrcthms)! - Added support for disabling 1Password integration in `TextField` - [#10406](#10406) [`87f559ca1`](87f559c) Thanks [@mrcthms](https://github.com/mrcthms)! - Updated the Toast component to support multiple re-renders being announced on screen readers ### Patch Changes - [#10428](#10428) [`d35d55e5b`](d35d55e) Thanks [@mrcthms](https://github.com/mrcthms)! - Fixed layout shift in `IndexFilters` when switching modes - Updated dependencies \[[`26ab8ce7d`](26ab8ce), [`7514dd1a3`](7514dd1), [`16c61668a`](16c6166)]: - @shopify/polaris-tokens@7.8.0 ## @shopify/polaris-tokens@7.8.0 ### Minor Changes - [#10432](#10432) [`26ab8ce7d`](26ab8ce) Thanks [@lgriffee](https://github.com/lgriffee)! - Added public primitive `border-radius` token scale - [#10448](#10448) [`7514dd1a3`](7514dd1) Thanks [@lgriffee](https://github.com/lgriffee)! - Added public primitive `space` token scale - [#10418](#10418) [`16c61668a`](16c6166) Thanks [@lgriffee](https://github.com/lgriffee)! - Added private primitive `size` token scale ## @shopify/polaris-migrator@0.22.3 ### Patch Changes - Updated dependencies \[[`26ab8ce7d`](26ab8ce), [`7514dd1a3`](7514dd1), [`16c61668a`](16c6166)]: - @shopify/polaris-tokens@7.8.0 - @shopify/stylelint-polaris@14.0.3 ## @shopify/stylelint-polaris@14.0.3 ### Patch Changes - Updated dependencies \[[`26ab8ce7d`](26ab8ce), [`7514dd1a3`](7514dd1), [`16c61668a`](16c6166)]: - @shopify/polaris-tokens@7.8.0 ## polaris.shopify.com@0.57.6 ### Patch Changes - Updated dependencies \[[`2e65a7e98`](2e65a7e), [`b7f25f07f`](b7f25f0), [`26ab8ce7d`](26ab8ce), [`d35d55e5b`](d35d55e), [`7514dd1a3`](7514dd1), [`16c61668a`](16c6166), [`87f559ca1`](87f559c)]: - @shopify/polaris@11.18.0 - @shopify/polaris-tokens@7.8.0 Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

…spected (#10566) ### WHY are these changes introduced? Navigating through the admin this morning I noticed an issue on the Orders index, when refreshing the page with a set of filters that don't match any existing filter, that the active filters don't appear in the Filters component. See video below: https://github.com/Shopify/polaris/assets/2562596/bb170b1f-0739-4778-8474-54c1ffe29f6e It turns out that the Orders index async loads the filter data if it does not match a saved view, and returns an empty `appliedFilters` array initially, before updating it with the correct `appliedFilters`. Due to [this recently introduced change](#10429) (sorry folks 🤡) we no longer are able to react to changes to the `appliedFilters` prop because of [this line](https://github.com/Shopify/polaris/pull/10429/files#diff-f21b98ea8a1f295706a4342f1e9ce543562f6c47d6d0682367e90bb7c23961d8R167), which sets the `localPinnedFilters` state only when the component mounts. In the case of the Orders index with the parameters mentioned above, this would set 0 pinned filters, due to the `appliedFilters` being empty. Because we don't call `setLocalPinnedFilters` on changes to the `appliedFilters`, this gives us the bug outlined in the video above. The filters do get set, but the filter does not appear in the pinned list. ### WHAT is this pull request doing? This PR effectively reverts that change, and instead uses a more robust approach to ensure that FilterPills being powered by `appliedFilters` rather than `localPinnedState` do not vanish when they become unapplied (when the filter values for the filter are removed). What we want to do, is add any applied filters to the array of `localPinnedState` so that when any filter value gets removed, the FilterPill remains as an unselected FilterPill. The merchant can then click out of the Popover to fully remove the FilterPill from the Filters bar. This is achieved by a `useEffect` hook, which listens for changes to the `appliedFilterKeys` and `localPinnedFilters` arrays, and checks if all values within `appliedFilterKeys` are not currently in the `localPinnedFilters` array. If that is the case, we know that we want to add these keys to the `localPinnedFilters` array, to ensure they are added as `FilterPill`s if the `appliedFilters` value changes post-mount. ### How to 🎩 🖥 [Local development instructions](https://github.com/Shopify/polaris/blob/main/README.md#local-development) 🗒 [General tophatting guidelines](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md) 📄 [Changelog guidelines](https://github.com/Shopify/polaris/blob/main/.github/CONTRIBUTING.md#changelog) Video of Snapshot in use on Spinstance: https://github.com/Shopify/polaris/assets/2562596/23decb34-c423-412c-90f0-213d89834786 - Spinstance: https://admin.web.orders-pill-check.marc-thomas.eu.spin.dev/store/shop1/orders?inContextTimeframe=none - Note that adding in filters that don't match a saved view, and then refreshing, the applied filters appear as FilterPills. - Storybook: https://5d559397bae39100201eedc1-kqkexazlai.chromatic.com/?path=/story/all-components-indexfilters--with-no-pinned-and-prefilled-filters&globals=polarisSummerEditions2023:true - Note that removing the values from the "Account status" filter keeps the filter pill visible, until the Popover is closed ### 🎩 checklist - [x] Tested on [mobile](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md#cross-browser-testing) - [x] Tested on [multiple browsers](https://help.shopify.com/en/manual/shopify-admin/supported-browsers) - [x] Tested for [accessibility](https://github.com/Shopify/polaris/blob/main/documentation/Accessibility%20testing.md) - [x] Updated the component's `README.md` with documentation changes - [x] [Tophatted documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md) changes in the style guide --------- Co-authored-by: Chloe Rice <chloerice@users.noreply.github.com>

### WHY are these changes introduced? Fixes an issue where if you have an applied filter set from the props when the component initiates, and then you remove the selected filter option, the filter pill disappears from the view. The correct behaviour is that the filter pill should remain, but be empty. ### Video of existing issue https://github.com/Shopify/polaris/assets/2562596/880c0974-23ce-4f8c-bda4-5734e7c8faa3 ### Video showing new intended behaviour https://github.com/Shopify/polaris/assets/2562596/b31b60b4-2077-4982-8545-bc54e03f1e9c ### WHAT is this pull request doing? The issue stemmed from having two sources of local pinned filters, and them behaving in different ways. Now, by only using the `localPinnedFilters` value, and setting the initial state value on component mount, we can expect the component to behave identically for both filter pills that are rendered on mount, and filter pills that are rendered as a result of a state change within the component. ### How to 🎩 🖥 [Local development instructions](https://github.com/Shopify/polaris/blob/main/README.md#local-development) 🗒 [General tophatting guidelines](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md) 📄 [Changelog guidelines](https://github.com/Shopify/polaris/blob/main/.github/CONTRIBUTING.md#changelog) ### 🎩 checklist - [x] Tested on [mobile](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md#cross-browser-testing) - [x] Tested on [multiple browsers](https://help.shopify.com/en/manual/shopify-admin/supported-browsers) - [x] Tested for [accessibility](https://github.com/Shopify/polaris/blob/main/documentation/Accessibility%20testing.md) - [x] Updated the component's `README.md` with documentation changes - [x] [Tophatted documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md) changes in the style guide --------- Co-authored-by: Chloe Rice <chloerice@users.noreply.github.com>

@mrcthms

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/polaris@11.18.0 ### Minor Changes - [Shopify#10429](Shopify#10429) [`2e65a7e98`](Shopify@78e3c64) Thanks [@mrcthms](https://github.com/mrcthms)! - Fixed `Filters` pinned filter pill not remaining when applied values are cleared - [Shopify#10394](Shopify#10394) [`b7f25f07f`](Shopify@11a5ec4) Thanks [@mrcthms](https://github.com/mrcthms)! - Added support for disabling 1Password integration in `TextField` - [Shopify#10406](Shopify#10406) [`87f559ca1`](Shopify@87ef9b6) Thanks [@mrcthms](https://github.com/mrcthms)! - Updated the Toast component to support multiple re-renders being announced on screen readers ### Patch Changes - [Shopify#10428](Shopify#10428) [`d35d55e5b`](Shopify@ded82d4) Thanks [@mrcthms](https://github.com/mrcthms)! - Fixed layout shift in `IndexFilters` when switching modes - Updated dependencies \[[`26ab8ce7d`](Shopify@2638248), [`7514dd1a3`](Shopify@beb664a), [`16c61668a`](Shopify@54284eb)]: - @shopify/polaris-tokens@7.8.0 ## @shopify/polaris-tokens@7.8.0 ### Minor Changes - [Shopify#10432](Shopify#10432) [`26ab8ce7d`](Shopify@2638248) Thanks [@lgriffee](https://github.com/lgriffee)! - Added public primitive `border-radius` token scale - [Shopify#10448](Shopify#10448) [`7514dd1a3`](Shopify@beb664a) Thanks [@lgriffee](https://github.com/lgriffee)! - Added public primitive `space` token scale - [Shopify#10418](Shopify#10418) [`16c61668a`](Shopify@54284eb) Thanks [@lgriffee](https://github.com/lgriffee)! - Added private primitive `size` token scale ## @shopify/polaris-migrator@0.22.3 ### Patch Changes - Updated dependencies \[[`26ab8ce7d`](Shopify@2638248), [`7514dd1a3`](Shopify@beb664a), [`16c61668a`](Shopify@54284eb)]: - @shopify/polaris-tokens@7.8.0 - @shopify/stylelint-polaris@14.0.3 ## @shopify/stylelint-polaris@14.0.3 ### Patch Changes - Updated dependencies \[[`26ab8ce7d`](Shopify@2638248), [`7514dd1a3`](Shopify@beb664a), [`16c61668a`](Shopify@54284eb)]: - @shopify/polaris-tokens@7.8.0 ## polaris.shopify.com@0.57.6 ### Patch Changes - Updated dependencies \[[`2e65a7e98`](Shopify@78e3c64), [`b7f25f07f`](Shopify@11a5ec4), [`26ab8ce7d`](Shopify@2638248), [`d35d55e5b`](Shopify@ded82d4), [`7514dd1a3`](Shopify@beb664a), [`16c61668a`](Shopify@54284eb), [`87f559ca1`](Shopify@87ef9b6)]: - @shopify/polaris@11.18.0 - @shopify/polaris-tokens@7.8.0 Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

…spected (Shopify#10566) ### WHY are these changes introduced? Navigating through the admin this morning I noticed an issue on the Orders index, when refreshing the page with a set of filters that don't match any existing filter, that the active filters don't appear in the Filters component. See video below: https://github.com/Shopify/polaris/assets/2562596/bb170b1f-0739-4778-8474-54c1ffe29f6e It turns out that the Orders index async loads the filter data if it does not match a saved view, and returns an empty `appliedFilters` array initially, before updating it with the correct `appliedFilters`. Due to [this recently introduced change](Shopify#10429) (sorry folks 🤡) we no longer are able to react to changes to the `appliedFilters` prop because of [this line](https://github.com/Shopify/polaris/pull/10429/files#diff-f21b98ea8a1f295706a4342f1e9ce543562f6c47d6d0682367e90bb7c23961d8R167), which sets the `localPinnedFilters` state only when the component mounts. In the case of the Orders index with the parameters mentioned above, this would set 0 pinned filters, due to the `appliedFilters` being empty. Because we don't call `setLocalPinnedFilters` on changes to the `appliedFilters`, this gives us the bug outlined in the video above. The filters do get set, but the filter does not appear in the pinned list. ### WHAT is this pull request doing? This PR effectively reverts that change, and instead uses a more robust approach to ensure that FilterPills being powered by `appliedFilters` rather than `localPinnedState` do not vanish when they become unapplied (when the filter values for the filter are removed). What we want to do, is add any applied filters to the array of `localPinnedState` so that when any filter value gets removed, the FilterPill remains as an unselected FilterPill. The merchant can then click out of the Popover to fully remove the FilterPill from the Filters bar. This is achieved by a `useEffect` hook, which listens for changes to the `appliedFilterKeys` and `localPinnedFilters` arrays, and checks if all values within `appliedFilterKeys` are not currently in the `localPinnedFilters` array. If that is the case, we know that we want to add these keys to the `localPinnedFilters` array, to ensure they are added as `FilterPill`s if the `appliedFilters` value changes post-mount. ### How to 🎩 🖥 [Local development instructions](https://github.com/Shopify/polaris/blob/main/README.md#local-development) 🗒 [General tophatting guidelines](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md) 📄 [Changelog guidelines](https://github.com/Shopify/polaris/blob/main/.github/CONTRIBUTING.md#changelog) Video of Snapshot in use on Spinstance: https://github.com/Shopify/polaris/assets/2562596/23decb34-c423-412c-90f0-213d89834786 - Spinstance: https://admin.web.orders-pill-check.marc-thomas.eu.spin.dev/store/shop1/orders?inContextTimeframe=none - Note that adding in filters that don't match a saved view, and then refreshing, the applied filters appear as FilterPills. - Storybook: https://5d559397bae39100201eedc1-kqkexazlai.chromatic.com/?path=/story/all-components-indexfilters--with-no-pinned-and-prefilled-filters&globals=polarisSummerEditions2023:true - Note that removing the values from the "Account status" filter keeps the filter pill visible, until the Popover is closed ### 🎩 checklist - [x] Tested on [mobile](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md#cross-browser-testing) - [x] Tested on [multiple browsers](https://help.shopify.com/en/manual/shopify-admin/supported-browsers) - [x] Tested for [accessibility](https://github.com/Shopify/polaris/blob/main/documentation/Accessibility%20testing.md) - [x] Updated the component's `README.md` with documentation changes - [x] [Tophatted documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md) changes in the style guide --------- Co-authored-by: Chloe Rice <chloerice@users.noreply.github.com>

mrcthms added 2 commits September 13, 2023 15:03

chore: fix filters bug

2062859

chore: add changeset

e3704b0

chloerice approved these changes Sep 13, 2023

View reviewed changes

.changeset/beige-jeans-scream.md Outdated Show resolved Hide resolved

chloerice added Contribution Component labels Sep 13, 2023

Update .changeset/beige-jeans-scream.md

012b5db

Co-authored-by: Chloe Rice <chloerice@users.noreply.github.com>

jesstelford approved these changes Sep 14, 2023

View reviewed changes

mrcthms merged commit 2e65a7e into main Sep 14, 2023

mrcthms deleted the mrcthms-index-filters-filter-removal-bug branch September 14, 2023 08:42

shopify-github-actions-access bot mentioned this pull request Sep 14, 2023

Version Packages #10405

Merged

mrcthms mentioned this pull request Sep 19, 2023

[Filters] Fix bug where changes to appliedFilters prop don't get respected #10566

Merged

5 tasks

chloerice added 2023 Contribution tracking for 2023 Contribution Tracking contribution in conjuction with year and type, e.g., "Component" + "Contribution" + "2023" labels Dec 5, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Filters] Fix vanishing applied filters bug #10429

[Filters] Fix vanishing applied filters bug #10429

Uh oh!

mrcthms commented Sep 13, 2023

Uh oh!

mrcthms commented Sep 13, 2023

Uh oh!

github-actions bot commented Sep 13, 2023

Uh oh!

Uh oh!

Uh oh!

[Filters] Fix vanishing applied filters bug #10429

[Filters] Fix vanishing applied filters bug #10429

Uh oh!

Conversation

mrcthms commented Sep 13, 2023

WHY are these changes introduced?

Video of existing issue

Video showing new intended behaviour

WHAT is this pull request doing?

How to 🎩

🎩 checklist

Uh oh!

mrcthms commented Sep 13, 2023

Uh oh!

github-actions bot commented Sep 13, 2023

Uh oh!

Uh oh!

Uh oh!