Skip to content

Commit dd9cf5d

Browse files
authored
[FiltersBar] Fix vanishing filters (#11417)
### WHY are these changes introduced? Fixes Shopify/web#114283 We currently have a bug in the FiltersBar component, where if all filters are pinned on mount, and a merchant either clicks the "Clear all" button, or clears a filter individually, then that filter vanishes until the merchant refreshes the page. The cause of this was some missing logic to determine whether to show the Add filter button. Previously, it only checked for pinned filters on mount, and not for changes to the pinned filter state. ### How to 🎩 Spin URL: https://admin.web.vanishing-filters.marc-thomas.eu.spin.dev/store/shop1/collections?selectedView=all Story instance: https://5d559397bae39100201eedc1-epgxutgfws.chromatic.com/?path=/story/all-components-filters--with-all-filters-pinned ### 🎩 checklist - [x] Tested a [snapshot](https://github.com/Shopify/polaris/blob/main/documentation/Releasing.md#-snapshot-releases) - [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
1 parent f6f9da6 commit dd9cf5d

File tree

4 files changed

+292
-22
lines changed

4 files changed

+292
-22
lines changed

.changeset/serious-vans-sin.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@shopify/polaris': minor
3+
---
4+
5+
[FiltersBar] Fixed bug where filters would disappear from the FiltersBar when clicking the Clear all button

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

+201
Original file line numberDiff line numberDiff line change
@@ -1516,3 +1516,204 @@ export function WithFilterBarHidden() {
15161516
</div>
15171517
);
15181518
}
1519+
1520+
export function WithAllFiltersPinned() {
1521+
const [accountStatus, setAccountStatus] = useState(null);
1522+
const [moneySpent, setMoneySpent] = useState(null);
1523+
const [taggedWith, setTaggedWith] = useState('');
1524+
const [queryValue, setQueryValue] = useState('');
1525+
1526+
const handleAccountStatusChange = useCallback(
1527+
(value) => setAccountStatus(value),
1528+
[],
1529+
);
1530+
const handleMoneySpentChange = useCallback(
1531+
(value) => setMoneySpent(value),
1532+
[],
1533+
);
1534+
const handleTaggedWithChange = useCallback(
1535+
(value) => setTaggedWith(value),
1536+
[],
1537+
);
1538+
const handleFiltersQueryChange = useCallback(
1539+
(value) => setQueryValue(value),
1540+
[],
1541+
);
1542+
const handleAccountStatusRemove = useCallback(
1543+
() => setAccountStatus(null),
1544+
[],
1545+
);
1546+
const handleMoneySpentRemove = useCallback(() => setMoneySpent(null), []);
1547+
const handleTaggedWithRemove = useCallback(() => setTaggedWith(''), []);
1548+
const handleQueryValueRemove = useCallback(() => setQueryValue(''), []);
1549+
const handleFiltersClearAll = useCallback(() => {
1550+
handleAccountStatusRemove();
1551+
handleMoneySpentRemove();
1552+
handleTaggedWithRemove();
1553+
handleQueryValueRemove();
1554+
}, [
1555+
handleAccountStatusRemove,
1556+
handleMoneySpentRemove,
1557+
handleQueryValueRemove,
1558+
handleTaggedWithRemove,
1559+
]);
1560+
1561+
const filters = [
1562+
{
1563+
key: 'accountStatus',
1564+
label: 'Account status',
1565+
filter: (
1566+
<ChoiceList
1567+
title="Account status"
1568+
titleHidden
1569+
choices={[
1570+
{label: 'Enabled', value: 'enabled'},
1571+
{label: 'Not invited', value: 'not invited'},
1572+
{label: 'Invited', value: 'invited'},
1573+
{label: 'Declined', value: 'declined'},
1574+
]}
1575+
selected={accountStatus || []}
1576+
onChange={handleAccountStatusChange}
1577+
allowMultiple
1578+
/>
1579+
),
1580+
shortcut: true,
1581+
pinned: true,
1582+
},
1583+
{
1584+
key: 'taggedWith',
1585+
label: 'Tagged with',
1586+
filter: (
1587+
<TextField
1588+
label="Tagged with"
1589+
value={taggedWith}
1590+
onChange={handleTaggedWithChange}
1591+
autoComplete="off"
1592+
labelHidden
1593+
/>
1594+
),
1595+
shortcut: true,
1596+
pinned: true,
1597+
},
1598+
{
1599+
key: 'moneySpent',
1600+
label: 'Money spent',
1601+
filter: (
1602+
<RangeSlider
1603+
label="Money spent is between"
1604+
labelHidden
1605+
value={moneySpent || [0, 500]}
1606+
prefix="$"
1607+
output
1608+
min={0}
1609+
max={2000}
1610+
step={1}
1611+
onChange={handleMoneySpentChange}
1612+
/>
1613+
),
1614+
shortcut: true,
1615+
pinned: true,
1616+
},
1617+
];
1618+
1619+
const appliedFilters: FiltersProps['appliedFilters'] = [];
1620+
if (!isEmpty(accountStatus)) {
1621+
const key = 'accountStatus';
1622+
appliedFilters.push({
1623+
key,
1624+
label: disambiguateLabel(key, accountStatus),
1625+
onRemove: handleAccountStatusRemove,
1626+
});
1627+
}
1628+
if (!isEmpty(moneySpent)) {
1629+
const key = 'moneySpent';
1630+
appliedFilters.push({
1631+
key,
1632+
label: disambiguateLabel(key, moneySpent),
1633+
onRemove: handleMoneySpentRemove,
1634+
});
1635+
}
1636+
if (!isEmpty(taggedWith)) {
1637+
const key = 'taggedWith';
1638+
appliedFilters.push({
1639+
key,
1640+
label: disambiguateLabel(key, taggedWith),
1641+
onRemove: handleTaggedWithRemove,
1642+
});
1643+
}
1644+
1645+
return (
1646+
<div style={{height: '568px'}}>
1647+
<LegacyCard>
1648+
<ResourceList
1649+
resourceName={{singular: 'customer', plural: 'customers'}}
1650+
filterControl={
1651+
<Filters
1652+
queryValue={queryValue}
1653+
queryPlaceholder="Searching in all"
1654+
filters={filters}
1655+
appliedFilters={appliedFilters}
1656+
onQueryChange={handleFiltersQueryChange}
1657+
onQueryClear={handleQueryValueRemove}
1658+
onClearAll={handleFiltersClearAll}
1659+
/>
1660+
}
1661+
flushFilters
1662+
items={[
1663+
{
1664+
id: '341',
1665+
url: '#',
1666+
name: 'Mae Jemison',
1667+
location: 'Decatur, USA',
1668+
},
1669+
{
1670+
id: '256',
1671+
url: '#',
1672+
name: 'Ellen Ochoa',
1673+
location: 'Los Angeles, USA',
1674+
},
1675+
]}
1676+
renderItem={(item) => {
1677+
const {id, url, name, location} = item;
1678+
const media = <Avatar customer size="md" name={name} />;
1679+
1680+
return (
1681+
<ResourceList.Item
1682+
id={id}
1683+
url={url}
1684+
media={media}
1685+
accessibilityLabel={`View details for ${name}`}
1686+
>
1687+
<Text as="h3" fontWeight="bold">
1688+
{name}
1689+
</Text>
1690+
<div>{location}</div>
1691+
</ResourceList.Item>
1692+
);
1693+
}}
1694+
/>
1695+
</LegacyCard>
1696+
</div>
1697+
);
1698+
1699+
function disambiguateLabel(key, value) {
1700+
switch (key) {
1701+
case 'moneySpent':
1702+
return `Money spent is between $${value[0]} and $${value[1]}`;
1703+
case 'taggedWith':
1704+
return `Tagged with ${value}`;
1705+
case 'accountStatus':
1706+
return value.map((val) => `Customer ${val}`).join(', ');
1707+
default:
1708+
return value;
1709+
}
1710+
}
1711+
1712+
function isEmpty(value) {
1713+
if (Array.isArray(value)) {
1714+
return value.length === 0;
1715+
} else {
1716+
return value === '' || value == null;
1717+
}
1718+
}
1719+
}

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

+31-22
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,10 @@ export function FiltersBar({
7878
};
7979
const appliedFilterKeys = appliedFilters?.map(({key}) => key);
8080

81+
const pinnedFromPropsKeys = filters
82+
.filter(({pinned}) => pinned)
83+
.map(({key}) => key);
84+
8185
const pinnedFiltersFromPropsAndAppliedFilters = filters.filter(
8286
({pinned, key}) => {
8387
const isPinnedOrApplied =
@@ -182,17 +186,23 @@ export function FiltersBar({
182186
);
183187

184188
const handleClearAllFilters = () => {
185-
setLocalPinnedFilters([]);
189+
setLocalPinnedFilters(pinnedFromPropsKeys);
186190
onClearAll?.();
187191
};
188-
const shouldShowAddButton = filters.some((filter) => !filter.pinned);
192+
const shouldShowAddButton =
193+
filters.some((filter) => !filter.pinned) ||
194+
filters.length !== localPinnedFilters.length;
189195

190196
const pinnedFiltersMarkup = pinnedFilters.map(
191197
({key: filterKey, ...pinnedFilter}) => {
192198
const appliedFilter = appliedFilters?.find(({key}) => key === filterKey);
193199
const handleFilterPillRemove = () => {
194200
setLocalPinnedFilters((currentLocalPinnedFilters) =>
195-
currentLocalPinnedFilters.filter((key) => key !== filterKey),
201+
currentLocalPinnedFilters.filter((key) => {
202+
const isMatchedFilters = key === filterKey;
203+
const isPinnedFilterFromProps = pinnedFromPropsKeys.includes(key);
204+
return !isMatchedFilters || isPinnedFilterFromProps;
205+
}),
196206
);
197207
appliedFilter?.onRemove(filterKey);
198208
};
@@ -236,26 +246,25 @@ export function FiltersBar({
236246
</div>
237247
) : null;
238248

239-
const clearAllMarkup =
240-
appliedFilters?.length || localPinnedFilters.length ? (
241-
<div
242-
className={classNames(
243-
styles.ClearAll,
244-
hasOneOrMorePinnedFilters &&
245-
shouldShowAddButton &&
246-
styles.MultiplePinnedFilterClearAll,
247-
)}
249+
const clearAllMarkup = appliedFilters?.length ? (
250+
<div
251+
className={classNames(
252+
styles.ClearAll,
253+
hasOneOrMorePinnedFilters &&
254+
shouldShowAddButton &&
255+
styles.MultiplePinnedFilterClearAll,
256+
)}
257+
>
258+
<Button
259+
size="micro"
260+
onClick={handleClearAllFilters}
261+
removeUnderline
262+
variant="monochromePlain"
248263
>
249-
<Button
250-
size="micro"
251-
onClick={handleClearAllFilters}
252-
removeUnderline
253-
variant="monochromePlain"
254-
>
255-
{i18n.translate('Polaris.Filters.clearFilters')}
256-
</Button>
257-
</div>
258-
) : null;
264+
{i18n.translate('Polaris.Filters.clearFilters')}
265+
</Button>
266+
</div>
267+
) : null;
259268

260269
return (
261270
<div

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

+55
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {ActionList} from '../../../../ActionList';
66
import {FiltersBar} from '../FiltersBar';
77
import type {FiltersBarProps} from '../FiltersBar';
88
import {FilterPill} from '../../FilterPill';
9+
import {Button} from '../../../../Button';
910

1011
describe('<FiltersBar />', () => {
1112
let originalScroll: any;
@@ -388,4 +389,58 @@ describe('<FiltersBar />', () => {
388389
],
389390
});
390391
});
392+
393+
it('will keep a pinned filter from props pinned when clearing', () => {
394+
const appliedFilters = [
395+
{
396+
...defaultProps.filters[1],
397+
label: 'Bar 2',
398+
value: ['Bar 2'],
399+
onRemove: jest.fn(),
400+
},
401+
];
402+
const scrollSpy = jest.fn();
403+
HTMLElement.prototype.scroll = scrollSpy;
404+
const wrapper = mountWithApp(
405+
<FiltersBar {...defaultProps} appliedFilters={appliedFilters} />,
406+
);
407+
408+
wrapper
409+
.find(FilterPill, {
410+
label: 'Bar 2',
411+
})!
412+
.trigger('onRemove');
413+
414+
expect(wrapper).toContainReactComponentTimes(FilterPill, 1);
415+
});
416+
417+
it('will keep a pinned filter from props pinned when clearing all', () => {
418+
const appliedFilters = [
419+
{
420+
...defaultProps.filters[0],
421+
label: 'Bar 2',
422+
value: ['Bar 2'],
423+
onRemove: jest.fn(),
424+
},
425+
{
426+
...defaultProps.filters[2],
427+
label: 'Bar 2',
428+
value: ['Bar 2'],
429+
onRemove: jest.fn(),
430+
},
431+
];
432+
const scrollSpy = jest.fn();
433+
HTMLElement.prototype.scroll = scrollSpy;
434+
const wrapper = mountWithApp(
435+
<FiltersBar {...defaultProps} appliedFilters={appliedFilters} />,
436+
);
437+
438+
wrapper
439+
.find(Button, {
440+
children: 'Clear all',
441+
})!
442+
.trigger('onClick');
443+
444+
expect(wrapper).toContainReactComponentTimes(FilterPill, 1);
445+
});
391446
});

0 commit comments

Comments
 (0)