Skip to content

Commit 17fa970

Browse files
authored
[Backdrop] Call onClick regardless of setClosing prop presence (#8237)
<!-- ☝️How to write a good PR title: - Prefix it with [ComponentName] (if applicable), for example: [Button] - Start with a verb, for example: Add, Delete, Improve, Fix… - Give as much context as necessary and as little as possible - Prefix it with [WIP] while it’s a work in progress --> ### WHY are these changes introduced? I believe #8090 introduced a regression where the `onClick` prop of the `Backdrop` component is not called when the consumer of the component also doesn't provide `setClosing` as a prop (released in Polaris `10.23.0`). There are various occurrences of Web Admin using `Backdrop` without providing `setClosing` as a prop, so some sheets are not properly closing when clicking outside. For example in Customer Segmentation: <img width="1428" alt="Screenshot 2023-02-01 at 1 40 45 PM" src="https://user-images.githubusercontent.com/3925905/216133586-92b760bf-57c2-4bfd-9f46-3382d8f935c2.png"> You can reproduce the issue in this [CodeSandbox](https://codesandbox.io/s/condescending-cannon-7om9z7?file=/App.js:115-155). Switching to a version of Polaris < `10.23.0` fixes the issue. ### WHAT is this pull request doing? This PR ensures `onClick` is always called, regardless of `setClosing` being provided. ### 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) <!-- Give as much information as needed to experiment with the component in the playground. --> See Playground code. After opening the backdrop, clicking on it again should properly close it. <details> <summary>Copy-paste this code in <code>playground/Playground.tsx</code>:</summary> ```jsx import React, {useState} from 'react'; import {Backdrop, Button, Page} from '../src'; export function Playground() { const [open, setOpen] = useState(false); return ( <Page title="Playground"> <div style={{ position: "absolute", zIndex: 999999 }}> <Button onClick={() => setOpen(!open)}>Toggle backdrop</Button> </div> {open && ( <Backdrop setClosing={() => {}} onClick={() => { console.log("onClick from Backdrop"); setOpen(false); }} /> )} </Page> ); } ``` </details> You can also 🎩 directly in Admin using this [Spin instance](https://admin.web.customer-data-platform-ujet.loic-delaubier.us.spin.dev/store/shop1/customers): - Go to the Customers section - Open Segmentation templates <img width="1189" alt="Screenshot 2023-02-01 at 2 18 18 PM" src="https://user-images.githubusercontent.com/3925905/216141402-c395b9df-8990-4315-84ca-c88607b759fd.png"> - Click outside of the sheet. The sheet should properly close. ### 🎩 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) - [ ] Tested for [accessibility](https://github.com/Shopify/polaris/blob/main/documentation/Accessibility%20testing.md) - [ ] Updated the component's `README.md` with documentation changes - [ ] [Tophatted documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md) changes in the style guide
1 parent 5fbe754 commit 17fa970

File tree

3 files changed

+19
-4
lines changed

3 files changed

+19
-4
lines changed

.changeset/unlucky-bags-fry.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@shopify/polaris': patch
3+
---
4+
5+
Fixed Backdrop onClick callback when setClosing is missing

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,11 @@ export function Backdrop(props: BackdropProps) {
3030
};
3131

3232
const handleClick = () => {
33-
if (setClosing && onClick) {
33+
if (setClosing) {
3434
setClosing(false);
35+
}
36+
37+
if (onClick) {
3538
onClick();
3639
}
3740
};

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

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,16 @@ describe('<Backdrop />', () => {
77
describe('onClick()', () => {
88
it('is called when the backdrop is clicked', () => {
99
const spy = jest.fn();
10-
const backdrop = mountWithApp(
11-
<Backdrop onClick={spy} setClosing={() => {}} />,
12-
);
10+
const backdrop = mountWithApp(<Backdrop onClick={spy} />);
11+
backdrop.find('div')!.trigger('onClick');
12+
expect(spy).toHaveBeenCalled();
13+
});
14+
});
15+
16+
describe('setClosing()', () => {
17+
it('is called when the backdrop is clicked', () => {
18+
const spy = jest.fn();
19+
const backdrop = mountWithApp(<Backdrop setClosing={spy} />);
1320
backdrop.find('div')!.trigger('onClick');
1421
expect(spy).toHaveBeenCalled();
1522
});

0 commit comments

Comments
 (0)