Skip to content

Commit 7691259

Browse files
alex-pagealex-page
committed
Merge branch 'main' into v11-major
2 parents 3bc5ac9 + 69d27db commit 7691259

File tree

7 files changed

+181
-99
lines changed

7 files changed

+181
-99
lines changed

‎.changeset/brown-gifts-notice.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 focus not returning to the `Popover` `activator` on keypress of Escape

‎.changeset/kind-jobs-beg.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@shopify/stylelint-polaris': patch
3+
---
4+
5+
Re-enable non layout component related layout rules

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

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,26 +25,50 @@ export default {
2525
} as ComponentMeta<typeof Popover>;
2626

2727
export function WithActionList() {
28-
const [popoverActive, setPopoverActive] = useState(true);
28+
const [activePopover, setActivePopover] = useState(null);
2929

30-
const togglePopoverActive = useCallback(
31-
() => setPopoverActive((popoverActive) => !popoverActive),
32-
[],
33-
);
30+
const togglePopoverActive = useCallback((popover, isClosing) => {
31+
const currentPopover = isClosing ? null : popover;
32+
setActivePopover(currentPopover);
33+
}, []);
3434

3535
const activator = (
36-
<Button onClick={togglePopoverActive} disclosure>
36+
<Button
37+
id="button-1"
38+
onClick={() => togglePopoverActive('popover1', false)}
39+
disclosure
40+
>
41+
More actions
42+
</Button>
43+
);
44+
const activator2 = (
45+
<Button
46+
id="button-2"
47+
onClick={() => togglePopoverActive('popover2', false)}
48+
disclosure
49+
>
3750
More actions
3851
</Button>
3952
);
4053

4154
return (
4255
<div style={{height: '250px'}}>
4356
<Popover
44-
active={popoverActive}
57+
active={activePopover === 'popover1'}
4558
activator={activator}
4659
autofocusTarget="first-node"
47-
onClose={togglePopoverActive}
60+
onClose={() => togglePopoverActive('popover1', true)}
61+
>
62+
<ActionList
63+
actionRole="menuitem"
64+
items={[{content: 'Import'}, {content: 'Export'}]}
65+
/>
66+
</Popover>
67+
<Popover
68+
active={activePopover === 'popover2'}
69+
activator={activator2}
70+
autofocusTarget="first-node"
71+
onClose={() => togglePopoverActive('popover2', true)}
4872
>
4973
<ActionList
5074
actionRole="menuitem"

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

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -159,17 +159,27 @@ const PopoverComponent = forwardRef<PopoverPublicAPI, PopoverProps>(
159159
return;
160160
}
161161

162-
if (
163-
(source === PopoverCloseSource.FocusOut ||
164-
source === PopoverCloseSource.EscapeKeypress) &&
162+
if (source === PopoverCloseSource.FocusOut && activatorNode) {
163+
const focusableActivator =
164+
findFirstFocusableNodeIncludingDisabled(activatorNode) ||
165+
findFirstFocusableNodeIncludingDisabled(activatorContainer.current) ||
166+
activatorContainer.current;
167+
if (!focusNextFocusableNode(focusableActivator, isInPortal)) {
168+
focusableActivator.focus();
169+
}
170+
} else if (
171+
source === PopoverCloseSource.EscapeKeypress &&
165172
activatorNode
166173
) {
167174
const focusableActivator =
168175
findFirstFocusableNodeIncludingDisabled(activatorNode) ||
169176
findFirstFocusableNodeIncludingDisabled(activatorContainer.current) ||
170177
activatorContainer.current;
171-
if (!focusNextFocusableNode(focusableActivator, isInPortal)) {
178+
179+
if (focusableActivator) {
172180
focusableActivator.focus();
181+
} else {
182+
focusNextFocusableNode(focusableActivator, isInPortal);
173183
}
174184
}
175185
};

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

Lines changed: 40 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import {Portal} from '../../Portal';
55
import {PositionedOverlay} from '../../PositionedOverlay';
66
import {Popover} from '../Popover';
77
import type {PopoverPublicAPI} from '../Popover';
8-
import {Pane, PopoverOverlay} from '../components';
8+
import {Pane, PopoverCloseSource, PopoverOverlay} from '../components';
99
import * as setActivatorAttributes from '../set-activator-attributes';
1010
import {TextContainer} from '../../TextContainer';
1111

@@ -273,23 +273,56 @@ describe('<Popover />', () => {
273273
expect(onCloseSpy).not.toHaveBeenCalled();
274274
});
275275

276-
it('focuses the next available element when the popover is closed', () => {
277-
const id = 'focus-target';
276+
it('focuses the next available element when the popover is closed using the tab key', () => {
277+
const activatorId = 'focus-target';
278+
const nextFocusedId = 'focus-target2';
278279
function PopoverTest() {
279280
return (
280281
<>
281282
<div>
282-
<Popover active activator={<div />} onClose={noop} />
283+
<Popover
284+
active
285+
activator={<button id={activatorId} />}
286+
onClose={noop}
287+
/>
283288
</div>
284-
<button id={id} />
289+
<button id={nextFocusedId} />
285290
</>
286291
);
287292
}
288293

289294
const popover = mountWithApp(<PopoverTest />);
295+
popover
296+
.find(PopoverOverlay)
297+
?.trigger('onClose', PopoverCloseSource.FocusOut);
298+
const focusTarget = popover.find('button', {id: nextFocusedId})!.domNode;
290299

291-
popover.find(PopoverOverlay)!.trigger('onClose', 1);
292-
const focusTarget = popover.find('button', {id})!.domNode;
300+
expect(document.activeElement).toBe(focusTarget);
301+
});
302+
303+
it('focuses the initial element that activated the popover when the popover is closed using the esc key', () => {
304+
const activatorId = 'focus-target';
305+
const nextFocusedId = 'focus-target2';
306+
function PopoverTest() {
307+
return (
308+
<>
309+
<div>
310+
<Popover
311+
active
312+
activator={<button id={activatorId} />}
313+
onClose={noop}
314+
/>
315+
</div>
316+
<button id={nextFocusedId} />
317+
</>
318+
);
319+
}
320+
321+
const popover = mountWithApp(<PopoverTest />);
322+
popover
323+
.find(PopoverOverlay)
324+
?.trigger('onClose', PopoverCloseSource.EscapeKeypress);
325+
const focusTarget = popover.find('button', {id: activatorId})!.domNode;
293326

294327
expect(document.activeElement).toBe(focusTarget);
295328
});

‎polaris.shopify.com/pages/tools/stylelint-polaris/rules/index.tsx‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -84,14 +84,19 @@ function ruleListMarkdown(): string {
8484
content[category] = ['', `## ${category}`, ''];
8585
}
8686

87+
// Temporary removal of layout rules until it is re-enabled
88+
// https://github.com/Shopify/polaris/issues/8188
89+
if (
90+
title.includes('layout/declaration-property-value-disallowed-list') ||
91+
title.includes('layout/property-disallowed-list')
92+
) {
93+
return;
94+
}
95+
8796
content[category].push(`- [${title}](${url}): ${description}`);
8897
}
8998
});
9099

91-
// Temporary removal of layout rules until it is re-enabled
92-
// https://github.com/Shopify/polaris/issues/8188
93-
delete content['Layout'];
94-
95100
const ruleList: string[] = Object.keys(content).reduce(
96101
(prev: string[], key: string) => [...prev, ...content[key]],
97102
[],

‎stylelint-polaris/index.js‎

Lines changed: 76 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -140,82 +140,82 @@ const stylelintPolarisCoverageOptions = {
140140
message: 'Please use a Polaris font token or typography component',
141141
},
142142
],
143-
// layout: [
144-
// {
145-
// 'declaration-property-value-disallowed-list': [
146-
// {
147-
// top: [/(?!var\(--p-).+$/],
148-
// bottom: [/(?!var\(--p-).+$/],
149-
// left: [/(?!var\(--p-).+$/],
150-
// right: [/(?!var\(--p-).+$/],
151-
// '/^width/': [/(?!var\(--p-).+$/],
152-
// '/^height/': [/(?!var\(--p-).+$/],
153-
// },
154-
// {severity: 'warning'},
155-
// ],
156-
// 'property-disallowed-list': [
157-
// [
158-
// 'position',
159-
// 'grid',
160-
// 'flex',
161-
// 'flex-grow',
162-
// 'flex-shrink',
163-
// 'flex-basis',
164-
// 'justify-content',
165-
// 'align-items',
166-
// 'grid-row',
167-
// 'grid-row-start',
168-
// 'grid-row-end',
169-
// 'grid-column',
170-
// 'grid-column-start',
171-
// 'grid-column-end',
172-
// 'grid-template',
173-
// 'grid-template-areas',
174-
// 'grid-template-rows',
175-
// 'grid-template-columns',
176-
// 'grid-area',
177-
// 'display',
178-
// ],
179-
// {severity: 'warning'},
180-
// ],
181-
// 'function-disallowed-list': [
182-
// 'nav-min-window-corrected',
183-
// 'control-height',
184-
// 'control-slim-height',
185-
// 'mobile-nav-width',
186-
// 'thumbnail-size',
187-
// 'icon-size',
188-
// 'top-bar-height',
189-
// ].map(matchNameRegExp),
190-
// 'polaris/at-rule-disallowed-list': {
191-
// include: [
192-
// 'layout-flex-fix',
193-
// 'safe-area-for',
194-
// 'skeleton-page-header-layout',
195-
// 'skeleton-page-secondary-actions-layout',
196-
// ].map(matchNameRegExp),
197-
// },
198-
// 'polaris/global-disallowed-list': [
199-
// // Legacy mixin map-get data
200-
// /\$layout-width-data/,
201-
// /\$navigation-width/,
202-
// /\$small-thumbnail-size/,
203-
// /\$large-thumbnail-size/,
204-
// /\$medium-thumbnail-size/,
205-
// /\$thumbnail-sizes/,
206-
// // Legacy custom properties
207-
// /--p-range-slider-thumb-size-base/,
208-
// /--p-range-slider-thumb-size-active/,
209-
// /--p-range-slider-thumb-scale/,
210-
// /--p-override-visible/,
211-
// /--p-icon-size/,
212-
// /--p-choice-size/,
213-
// ],
214-
// },
215-
// {
216-
// message: 'Please use a Polaris layout component',
217-
// },
218-
// ],
143+
layout: [
144+
{
145+
// 'declaration-property-value-disallowed-list': [
146+
// {
147+
// top: [/(?!var\(--p-).+$/],
148+
// bottom: [/(?!var\(--p-).+$/],
149+
// left: [/(?!var\(--p-).+$/],
150+
// right: [/(?!var\(--p-).+$/],
151+
// '/^width/': [/(?!var\(--p-).+$/],
152+
// '/^height/': [/(?!var\(--p-).+$/],
153+
// },
154+
// {severity: 'warning'},
155+
// ],
156+
// 'property-disallowed-list': [
157+
// [
158+
// 'position',
159+
// 'grid',
160+
// 'flex',
161+
// 'flex-grow',
162+
// 'flex-shrink',
163+
// 'flex-basis',
164+
// 'justify-content',
165+
// 'align-items',
166+
// 'grid-row',
167+
// 'grid-row-start',
168+
// 'grid-row-end',
169+
// 'grid-column',
170+
// 'grid-column-start',
171+
// 'grid-column-end',
172+
// 'grid-template',
173+
// 'grid-template-areas',
174+
// 'grid-template-rows',
175+
// 'grid-template-columns',
176+
// 'grid-area',
177+
// 'display',
178+
// ],
179+
// {severity: 'warning'},
180+
// ],
181+
'function-disallowed-list': [
182+
'nav-min-window-corrected',
183+
'control-height',
184+
'control-slim-height',
185+
'mobile-nav-width',
186+
'thumbnail-size',
187+
'icon-size',
188+
'top-bar-height',
189+
].map(matchNameRegExp),
190+
'polaris/at-rule-disallowed-list': {
191+
include: [
192+
'layout-flex-fix',
193+
'safe-area-for',
194+
'skeleton-page-header-layout',
195+
'skeleton-page-secondary-actions-layout',
196+
].map(matchNameRegExp),
197+
},
198+
'polaris/global-disallowed-list': [
199+
// Legacy mixin map-get data
200+
/\$layout-width-data/,
201+
/\$navigation-width/,
202+
/\$small-thumbnail-size/,
203+
/\$large-thumbnail-size/,
204+
/\$medium-thumbnail-size/,
205+
/\$thumbnail-sizes/,
206+
// Legacy custom properties
207+
/--p-range-slider-thumb-size-base/,
208+
/--p-range-slider-thumb-size-active/,
209+
/--p-range-slider-thumb-scale/,
210+
/--p-override-visible/,
211+
/--p-icon-size/,
212+
/--p-choice-size/,
213+
],
214+
},
215+
{
216+
message: 'Please use a Polaris layout component',
217+
},
218+
],
219219
spacing: [
220220
{
221221
'function-disallowed-list': ['control-vertical-padding', 'spacing'].map(

0 commit comments

Comments
 (0)