Skip to content

Commit c1c8f73

Browse files
[Popover] Add captureOverscroll prop (#7588)
<!-- ☝️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? When a user scrolls to the bottom of the feed, we don't want the rest of the page to scroll. Fixes [#1206](Shopify/merchant-communication#1206) <!-- link to issue if one exists --> <!-- Context about the problem that’s being addressed. --> ### WHAT is this pull request doing? <!-- Summary of the changes committed. Before / after screenshots are appreciated for UI changes. Make sure to include alt text that describes the screenshot. If you include an animated gif showing your change, wrapping it in a details tag is recommended. Gifs usually autoplay, which can cause accessibility issues for people reviewing your PR: <details> <summary>Summary of your gif(s)</summary> <img src="..." alt="Description of what the gif shows"> </details> --> This PR adds an optional `captureOverscroll` prop to the `Popover` component, which gets passed to its `Pane` component. When the prop is truthy, a class is added to the `Pane` component to capture the overscroll with this CSS: `overscroll-behavior: contain;`. The prop is set to `false` by default and will not impact any existing implementations. <details> <summary>Attempts to scroll past the end of the alerts feed do not cause the page to scroll</summary> https://user-images.githubusercontent.com/100380574/199125795-b6f90cf6-ce20-4a6e-b801-f658146c2c0e.mp4 </details> <!-- ℹ️ Delete the following for small / trivial changes --> ### 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. --> The updated version of the `Popover` component has been implemented on [this spin instance](https://admin.web.polaris-popover-update.rick-caplan.us.spin.dev/store/shop1). - [ ] Open the alerts feed by clicking on the bell icon - [ ] Scroll to the bottom of the feed to load the next set of results - [ ] Scroll to the bottom again and confirm attempts to scroll past the `No more alerts` message do not cause the page to scroll ### 🎩 checklist - [ ] Tested on [mobile](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md#cross-browser-testing) - [ ] 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 e1a3c62 commit c1c8f73

File tree

7 files changed

+138
-3
lines changed

7 files changed

+138
-3
lines changed

.changeset/strange-beans-grow.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+
Added optional `captureOverscroll` prop to `Popover`

polaris-react/src/components/Popover/Popover.scss

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,10 @@ $vertical-motion-offset: -5px;
106106
flex: 0 0 auto;
107107
}
108108

109+
.Pane-captureOverscroll {
110+
overscroll-behavior: contain;
111+
}
112+
109113
.Section {
110114
padding: var(--p-space-4);
111115

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,11 @@ export interface PopoverProps {
7878
autofocusTarget?: PopoverAutofocusTarget;
7979
/** Prevents closing the popover when other overlays are clicked */
8080
preventCloseOnChildOverlayClick?: boolean;
81+
/**
82+
* Prevents page scrolling when the end of the scrollable Popover overlay content is reached - applied to Pane subcomponent
83+
* @default false
84+
*/
85+
captureOverscroll?: boolean;
8186
}
8287

8388
export interface PopoverPublicAPI {

polaris-react/src/components/Popover/components/Pane/Pane.tsx

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,16 +17,26 @@ export interface PaneProps {
1717
height?: string;
1818
/** Callback when the bottom of the popover is reached by mouse or keyboard */
1919
onScrolledToBottom?(): void;
20+
/**
21+
* Prevents page scrolling when the end of the scrollable Popover content is reached
22+
* @default false
23+
*/
24+
captureOverscroll?: boolean;
2025
}
2126

2227
export function Pane({
28+
captureOverscroll = false,
2329
fixed,
2430
sectioned,
2531
children,
2632
height,
2733
onScrolledToBottom,
2834
}: PaneProps) {
29-
const className = classNames(styles.Pane, fixed && styles['Pane-fixed']);
35+
const className = classNames(
36+
styles.Pane,
37+
fixed && styles['Pane-fixed'],
38+
captureOverscroll && styles['Pane-captureOverscroll'],
39+
);
3040
const content = sectioned
3141
? wrapWithComponent(children, Section, {})
3242
: children;

polaris-react/src/components/Popover/components/Pane/tests/Pane.test.tsx

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,4 +153,54 @@ describe('<Pane />', () => {
153153
});
154154
});
155155
});
156+
157+
describe('captureOverscroll', () => {
158+
const Children = () => (
159+
<TextContainer>
160+
<p>Text</p>
161+
</TextContainer>
162+
);
163+
164+
describe('when not passed', () => {
165+
it('does not apply the Pane-captureOverscroll class', () => {
166+
const popoverPane = mountWithApp(
167+
<Pane>
168+
<Children />
169+
</Pane>,
170+
);
171+
172+
expect(popoverPane).toContainReactComponent(Scrollable, {
173+
className: 'Pane',
174+
});
175+
});
176+
});
177+
178+
describe('when passed as true', () => {
179+
it('applies the Pane-captureOverscroll class', () => {
180+
const popoverPane = mountWithApp(
181+
<Pane captureOverscroll>
182+
<Children />
183+
</Pane>,
184+
);
185+
186+
expect(popoverPane).toContainReactComponent(Scrollable, {
187+
className: 'Pane Pane-captureOverscroll',
188+
});
189+
});
190+
});
191+
192+
describe('when passed as false', () => {
193+
it('does not apply the Pane-captureOverscroll class', () => {
194+
const popoverPane = mountWithApp(
195+
<Pane captureOverscroll={false}>
196+
<Children />
197+
</Pane>,
198+
);
199+
200+
expect(popoverPane).toContainReactComponent(Scrollable, {
201+
className: 'Pane',
202+
});
203+
});
204+
});
205+
});
156206
});

polaris-react/src/components/Popover/components/PopoverOverlay/PopoverOverlay.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ export interface PopoverOverlayProps {
5757
onClose(source: PopoverCloseSource): void;
5858
autofocusTarget?: PopoverAutofocusTarget;
5959
preventCloseOnChildOverlayClick?: boolean;
60+
captureOverscroll?: boolean;
6061
}
6162

6263
interface State {
@@ -222,6 +223,7 @@ export class PopoverOverlay extends PureComponent<PopoverOverlayProps, State> {
222223
fluidContent,
223224
hideOnPrint,
224225
autofocusTarget,
226+
captureOverscroll,
225227
} = this.props;
226228

227229
const className = classNames(
@@ -248,7 +250,7 @@ export class PopoverOverlay extends PureComponent<PopoverOverlayProps, State> {
248250
style={contentStyles}
249251
ref={this.contentNode}
250252
>
251-
{renderPopoverContent(children, {sectioned})}
253+
{renderPopoverContent(children, {captureOverscroll, sectioned})}
252254
</div>
253255
);
254256

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

Lines changed: 60 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,9 @@ import {Portal} from '../../Portal';
55
import {PositionedOverlay} from '../../PositionedOverlay';
66
import {Popover} from '../Popover';
77
import type {PopoverPublicAPI} from '../Popover';
8-
import {PopoverOverlay} from '../components';
8+
import {Pane, PopoverOverlay} from '../components';
99
import * as setActivatorAttributes from '../set-activator-attributes';
10+
import {TextContainer} from '../../TextContainer';
1011

1112
describe('<Popover />', () => {
1213
const spy = jest.fn();
@@ -368,6 +369,64 @@ describe('<Popover />', () => {
368369
});
369370
});
370371
});
372+
373+
describe('captureOverscroll', () => {
374+
const TestActivator = <button>Activator</button>;
375+
376+
const Children = () => (
377+
<TextContainer>
378+
<p>Text</p>
379+
</TextContainer>
380+
);
381+
382+
const defaultProps = {
383+
active: true,
384+
activator: TestActivator,
385+
onClose: jest.fn(),
386+
};
387+
388+
describe('when not passed', () => {
389+
it('does not pass the prop as true to the Pane component', () => {
390+
const popover = mountWithApp(
391+
<Popover {...defaultProps}>
392+
<Children />
393+
</Popover>,
394+
);
395+
396+
expect(popover).toContainReactComponent(Pane, {
397+
captureOverscroll: undefined,
398+
});
399+
});
400+
});
401+
402+
describe('when passed as true', () => {
403+
it('passes the prop as true to the Pane component', () => {
404+
const popover = mountWithApp(
405+
<Popover {...defaultProps} captureOverscroll>
406+
<Children />
407+
</Popover>,
408+
);
409+
410+
expect(popover).toContainReactComponent(Pane, {
411+
captureOverscroll: true,
412+
});
413+
});
414+
});
415+
416+
describe('when passed as false', () => {
417+
it('passes the prop as false to the Pane component', () => {
418+
const popover = mountWithApp(
419+
<Popover {...defaultProps} captureOverscroll={false}>
420+
<Children />
421+
</Popover>,
422+
);
423+
424+
expect(popover).toContainReactComponent(Pane, {
425+
captureOverscroll: false,
426+
});
427+
});
428+
});
429+
});
371430
});
372431

373432
function noop() {}

0 commit comments

Comments
 (0)