Skip to content

Commit 2e65a7e

Browse files
mrcthmschloerice
andauthored
[Filters] Fix vanishing applied filters bug (#10429)
### 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>
1 parent d35d55e commit 2e65a7e

File tree

3 files changed

+40
-11
lines changed

3 files changed

+40
-11
lines changed

.changeset/beige-jeans-scream.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@shopify/polaris': minor
3+
---
4+
5+
Fixed `Filters` pinned filter pill not remaining when applied values are cleared

polaris-react/src/components/Filters/Filters.tsx

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,6 @@ export function Filters({
141141
const {mdDown} = useBreakpoints();
142142
const {polarisSummerEditions2023: se23} = useFeatures();
143143
const [popoverActive, setPopoverActive] = useState(false);
144-
const [localPinnedFilters, setLocalPinnedFilters] = useState<string[]>([]);
145144
const hasMounted = useRef(false);
146145

147146
useEffect(() => {
@@ -158,23 +157,23 @@ export function Filters({
158157
const appliedFilterKeys = appliedFilters?.map(({key}) => key);
159158

160159
const pinnedFiltersFromPropsAndAppliedFilters = filters.filter(
161-
({pinned, key}) =>
162-
(Boolean(pinned) || appliedFilterKeys?.includes(key)) &&
163-
// Filters that are pinned in local state display at the end of our list
164-
!localPinnedFilters.find((filterKey) => filterKey === key),
160+
({pinned, key}) => {
161+
const isPinnedOrApplied =
162+
Boolean(pinned) || appliedFilterKeys?.includes(key);
163+
return isPinnedOrApplied;
164+
},
165+
);
166+
const [localPinnedFilters, setLocalPinnedFilters] = useState<string[]>(
167+
pinnedFiltersFromPropsAndAppliedFilters.map(({key}) => key),
165168
);
166-
const pinnedFiltersFromLocalState = localPinnedFilters
169+
170+
const pinnedFilters = localPinnedFilters
167171
.map((key) => filters.find((filter) => filter.key === key))
168172
.reduce<FilterInterface[]>(
169173
(acc, filter) => (filter ? [...acc, filter] : acc),
170174
[],
171175
);
172176

173-
const pinnedFilters = [
174-
...pinnedFiltersFromPropsAndAppliedFilters,
175-
...pinnedFiltersFromLocalState,
176-
];
177-
178177
const onFilterClick =
179178
({key, onAction}: FilterInterface) =>
180179
() => {

polaris-react/src/components/Filters/tests/Filters.test.tsx

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,31 @@ describe('<Filters />', () => {
180180
expect(wrapper).toContainReactComponent(ActionList);
181181
});
182182

183+
it('will not remove a pinned filter when it is removed from the applied filters array', () => {
184+
const appliedFilters = [
185+
{
186+
...defaultProps.filters[2],
187+
label: 'Value is Baz',
188+
value: ['Baz'],
189+
onRemove: jest.fn(),
190+
},
191+
];
192+
const wrapper = mountWithApp(
193+
<Filters {...defaultProps} appliedFilters={appliedFilters} />,
194+
);
195+
196+
expect(wrapper.findAll(FilterPill)[1]).toHaveReactProps({
197+
label: 'Value is Baz',
198+
selected: true,
199+
});
200+
wrapper.setProps({appliedFilters: []});
201+
202+
expect(wrapper.findAll(FilterPill)[1]).toHaveReactProps({
203+
label: 'Baz',
204+
selected: false,
205+
});
206+
});
207+
183208
it('correctly invokes the onRemove callback when clicking on an applied filter', () => {
184209
const scrollSpy = jest.fn();
185210
HTMLElement.prototype.scroll = scrollSpy;

0 commit comments

Comments
 (0)