Skip to content

Commit 5b14429

Browse files
pan-kotpan-kot
authored
revert: Nested STSN providers support (#3781) (#3809)
1 parent bcd2b51 commit 5b14429

File tree

4 files changed

+30
-251
lines changed

4 files changed

+30
-251
lines changed

‎pages/table-fragments/grid-navigation-custom.page.tsx

Lines changed: 15 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import {
77
AppLayout,
88
Button,
99
ButtonDropdown,
10-
ButtonGroup,
1110
Checkbox,
1211
CollectionPreferences,
1312
ColumnLayout,
@@ -61,14 +60,13 @@ type PageContext = React.Context<
6160
}>
6261
>;
6362

64-
type ActionsMode = 'dropdown' | 'inline' | 'button-group';
63+
type ActionsMode = 'dropdown' | 'inline';
6564

6665
const tableRoleOptions = [{ value: 'table' }, { value: 'grid' }, { value: 'grid-default' }];
6766

6867
const actionsModeOptions = [
6968
{ value: 'dropdown', label: 'Dropdown' },
70-
{ value: 'inline', label: 'Inline' },
71-
{ value: 'button-group', label: 'Button group' },
69+
{ value: 'inline', label: 'Inline (anti-pattern)' },
7270
];
7371

7472
export default function Page() {
@@ -460,47 +458,19 @@ function ItemActionsCell({
460458
</div>
461459
);
462460
}
463-
if (mode === 'inline') {
464-
return (
465-
<div style={{ display: 'flex', gap: '8px', flexWrap: 'nowrap' }}>
466-
<Button variant="inline-icon" iconName="remove" ariaLabel="Delete item" onClick={onDelete} />
467-
<Button variant="inline-icon" iconName="copy" ariaLabel="Duplicate item" onClick={onDuplicate} />
468-
<Button
469-
variant="inline-icon"
470-
iconName="refresh"
471-
ariaLabel="Update item"
472-
onClick={onUpdate}
473-
disabled={!canUpdate}
474-
/>
475-
</div>
476-
);
477-
}
478-
if (mode === 'button-group') {
479-
return (
480-
<div style={{ inlineSize: 100 }}>
481-
<ButtonGroup
482-
ariaLabel="Item actions"
483-
variant="icon"
484-
items={[
485-
{ type: 'icon-button', id: 'delete', iconName: 'remove', text: 'Delete' },
486-
{ type: 'icon-button', id: 'duplicate', iconName: 'copy', text: 'Duplicate' },
487-
{ type: 'icon-button', id: 'update', iconName: 'refresh', text: 'Update', disabled: !canUpdate },
488-
]}
489-
onItemClick={event => {
490-
switch (event.detail.id) {
491-
case 'delete':
492-
return onDelete();
493-
case 'duplicate':
494-
return onDuplicate();
495-
case 'update':
496-
return onUpdate();
497-
}
498-
}}
499-
/>
500-
</div>
501-
);
502-
}
503-
return null;
461+
return (
462+
<div style={{ display: 'flex', gap: '8px', flexWrap: 'nowrap' }}>
463+
<Button variant="inline-icon" iconName="remove" ariaLabel="Delete item" onClick={onDelete} />
464+
<Button variant="inline-icon" iconName="copy" ariaLabel="Duplicate item" onClick={onDuplicate} />
465+
<Button
466+
variant="inline-icon"
467+
iconName="refresh"
468+
ariaLabel="Update item"
469+
onClick={onUpdate}
470+
disabled={!canUpdate}
471+
/>
472+
</div>
473+
);
504474
}
505475

506476
function DnsEditCell({ item }: { item: Instance }) {

‎src/internal/context/__tests__/single-tab-stop-navigation-context.test.tsx

Lines changed: 2 additions & 163 deletions
Original file line numberDiff line numberDiff line change
@@ -1,59 +1,21 @@
11
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
22
// SPDX-License-Identifier: Apache-2.0
33

4-
import React, { useContext, useEffect, useRef } from 'react';
4+
import React, { useRef } from 'react';
55
import { render } from '@testing-library/react';
66

77
import {
8-
SingleTabStopNavigationAPI,
98
SingleTabStopNavigationContext,
10-
SingleTabStopNavigationProvider,
119
useSingleTabStopNavigation,
1210
} from '../../../../lib/components/internal/context/single-tab-stop-navigation-context';
1311
import { renderWithSingleTabStopNavigation } from './utils';
1412

15-
// Simple STSN subscriber component
1613
function Button(props: React.HTMLAttributes<HTMLButtonElement>) {
1714
const buttonRef = useRef<HTMLButtonElement>(null);
1815
const { tabIndex } = useSingleTabStopNavigation(buttonRef, { tabIndex: props.tabIndex });
1916
return <button {...props} ref={buttonRef} tabIndex={tabIndex} />;
2017
}
2118

22-
// Simple STSN provider component
23-
function Group({
24-
id,
25-
navigationActive,
26-
children,
27-
}: {
28-
id: string;
29-
navigationActive: boolean;
30-
children: React.ReactNode;
31-
}) {
32-
const navigationAPI = useRef<SingleTabStopNavigationAPI>(null);
33-
34-
useEffect(() => {
35-
navigationAPI.current?.updateFocusTarget();
36-
});
37-
38-
return (
39-
<SingleTabStopNavigationProvider
40-
ref={navigationAPI}
41-
navigationActive={navigationActive}
42-
getNextFocusTarget={() => document.querySelector(`#${id}`)!.querySelectorAll('button')[0] as HTMLElement}
43-
>
44-
<div id={id}>
45-
<Button>First</Button>
46-
<Button>Second</Button>
47-
{children}
48-
</div>
49-
</SingleTabStopNavigationProvider>
50-
);
51-
}
52-
53-
function findGroupButton(groupId: string, buttonIndex: number) {
54-
return document.querySelector(`#${groupId}`)!.querySelectorAll('button')[buttonIndex] as HTMLElement;
55-
}
56-
5719
test('does not override tab index when keyboard navigation is not active', () => {
5820
renderWithSingleTabStopNavigation(<Button id="button" />, { navigationActive: false });
5921
expect(document.querySelector('#button')).not.toHaveAttribute('tabIndex');
@@ -113,9 +75,7 @@ test('propagates and suppresses navigation active state', () => {
11375
}
11476
function Test({ navigationActive }: { navigationActive: boolean }) {
11577
return (
116-
<SingleTabStopNavigationContext.Provider
117-
value={{ navigationActive, registerFocusable: () => () => {}, resetFocusTarget: () => {} }}
118-
>
78+
<SingleTabStopNavigationContext.Provider value={{ navigationActive, registerFocusable: () => () => {} }}>
11979
<Component />
12080
</SingleTabStopNavigationContext.Provider>
12181
);
@@ -127,124 +87,3 @@ test('propagates and suppresses navigation active state', () => {
12787
rerender(<Test navigationActive={false} />);
12888
expect(document.querySelector('div')).toHaveTextContent('false');
12989
});
130-
131-
test('subscriber components can be used without provider', () => {
132-
function TestComponent(props: React.HTMLAttributes<HTMLButtonElement>) {
133-
const ref = useRef(null);
134-
const contextResult = useContext(SingleTabStopNavigationContext);
135-
const hookResult = useSingleTabStopNavigation(ref, { tabIndex: props.tabIndex });
136-
useEffect(() => {
137-
contextResult.registerFocusable(ref.current!, () => {});
138-
contextResult.resetFocusTarget();
139-
});
140-
return (
141-
<div ref={ref}>
142-
Context: {`${contextResult.navigationActive}`}, Hook: {`${hookResult.navigationActive}:${hookResult.tabIndex}`}
143-
</div>
144-
);
145-
}
146-
const { container } = render(<TestComponent />);
147-
expect(container.textContent).toBe('Context: false, Hook: false:undefined');
148-
});
149-
150-
describe('nested contexts', () => {
151-
test('tab indices are distributed correctly when switching contexts from inner to outer', () => {
152-
const { rerender } = render(
153-
<Group id="outer-most" navigationActive={false}>
154-
<Group id="outer" navigationActive={false}>
155-
<Group id="inner" navigationActive={true}>
156-
{null}
157-
</Group>
158-
</Group>
159-
</Group>
160-
);
161-
expect(findGroupButton('outer-most', 0)).not.toHaveAttribute('tabindex');
162-
expect(findGroupButton('outer-most', 1)).not.toHaveAttribute('tabindex');
163-
expect(findGroupButton('outer', 0)).not.toHaveAttribute('tabindex');
164-
expect(findGroupButton('outer', 1)).not.toHaveAttribute('tabindex');
165-
expect(findGroupButton('inner', 0)).toHaveAttribute('tabindex', '0');
166-
expect(findGroupButton('inner', 1)).toHaveAttribute('tabindex', '-1');
167-
168-
rerender(
169-
<Group id="outer-most" navigationActive={false}>
170-
<Group id="outer" navigationActive={true}>
171-
<Group id="inner" navigationActive={true}>
172-
{null}
173-
</Group>
174-
</Group>
175-
</Group>
176-
);
177-
expect(findGroupButton('outer-most', 0)).not.toHaveAttribute('tabindex');
178-
expect(findGroupButton('outer-most', 1)).not.toHaveAttribute('tabindex');
179-
expect(findGroupButton('outer', 0)).toHaveAttribute('tabindex', '0');
180-
expect(findGroupButton('outer', 1)).toHaveAttribute('tabindex', '-1');
181-
expect(findGroupButton('inner', 0)).toHaveAttribute('tabindex', '-1');
182-
expect(findGroupButton('inner', 1)).toHaveAttribute('tabindex', '-1');
183-
184-
rerender(
185-
<Group id="outer-most" navigationActive={true}>
186-
<Group id="outer" navigationActive={true}>
187-
<Group id="inner" navigationActive={true}>
188-
{null}
189-
</Group>
190-
</Group>
191-
</Group>
192-
);
193-
expect(findGroupButton('outer-most', 0)).toHaveAttribute('tabindex', '0');
194-
expect(findGroupButton('outer-most', 1)).toHaveAttribute('tabindex', '-1');
195-
expect(findGroupButton('outer', 0)).toHaveAttribute('tabindex', '-1');
196-
expect(findGroupButton('outer', 1)).toHaveAttribute('tabindex', '-1');
197-
expect(findGroupButton('inner', 0)).toHaveAttribute('tabindex', '-1');
198-
expect(findGroupButton('inner', 1)).toHaveAttribute('tabindex', '-1');
199-
});
200-
201-
test('tab indices are distributed correctly when switching contexts from outer to inner', () => {
202-
const { rerender } = render(
203-
<Group id="outer-most" navigationActive={true}>
204-
<Group id="outer" navigationActive={true}>
205-
<Group id="inner" navigationActive={true}>
206-
{null}
207-
</Group>
208-
</Group>
209-
</Group>
210-
);
211-
expect(findGroupButton('outer-most', 0)).toHaveAttribute('tabindex', '0');
212-
expect(findGroupButton('outer-most', 1)).toHaveAttribute('tabindex', '-1');
213-
expect(findGroupButton('outer', 0)).toHaveAttribute('tabindex', '-1');
214-
expect(findGroupButton('outer', 1)).toHaveAttribute('tabindex', '-1');
215-
expect(findGroupButton('inner', 0)).toHaveAttribute('tabindex', '-1');
216-
expect(findGroupButton('inner', 1)).toHaveAttribute('tabindex', '-1');
217-
218-
rerender(
219-
<Group id="outer-most" navigationActive={false}>
220-
<Group id="outer" navigationActive={true}>
221-
<Group id="inner" navigationActive={true}>
222-
{null}
223-
</Group>
224-
</Group>
225-
</Group>
226-
);
227-
expect(findGroupButton('outer-most', 0)).not.toHaveAttribute('tabindex');
228-
expect(findGroupButton('outer-most', 1)).not.toHaveAttribute('tabindex');
229-
expect(findGroupButton('outer', 0)).toHaveAttribute('tabindex', '0');
230-
expect(findGroupButton('outer', 1)).toHaveAttribute('tabindex', '-1');
231-
expect(findGroupButton('inner', 0)).toHaveAttribute('tabindex', '-1');
232-
expect(findGroupButton('inner', 1)).toHaveAttribute('tabindex', '-1');
233-
234-
rerender(
235-
<Group id="outer-most" navigationActive={false}>
236-
<Group id="outer" navigationActive={false}>
237-
<Group id="inner" navigationActive={true}>
238-
{null}
239-
</Group>
240-
</Group>
241-
</Group>
242-
);
243-
expect(findGroupButton('outer-most', 0)).not.toHaveAttribute('tabindex');
244-
expect(findGroupButton('outer-most', 1)).not.toHaveAttribute('tabindex');
245-
expect(findGroupButton('outer', 0)).not.toHaveAttribute('tabindex');
246-
expect(findGroupButton('outer', 1)).not.toHaveAttribute('tabindex');
247-
expect(findGroupButton('inner', 0)).toHaveAttribute('tabindex', '0');
248-
expect(findGroupButton('inner', 1)).toHaveAttribute('tabindex', '-1');
249-
});
250-
});

‎src/internal/context/__tests__/utils.tsx

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,7 @@ const FakeSingleTabStopNavigationProvider = forwardRef(
3939
}));
4040

4141
return (
42-
<SingleTabStopNavigationContext.Provider
43-
value={{ registerFocusable, navigationActive, resetFocusTarget: () => {} }}
44-
>
42+
<SingleTabStopNavigationContext.Provider value={{ registerFocusable, navigationActive }}>
4543
{children}
4644
</SingleTabStopNavigationContext.Provider>
4745
);

‎src/internal/context/single-tab-stop-navigation-context.tsx

Lines changed: 12 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -11,19 +11,16 @@ import React, {
1111
useState,
1212
} from 'react';
1313

14-
import { useEffectOnUpdate } from '../hooks/use-effect-on-update';
1514
import { nodeBelongs } from '../utils/node-belongs';
1615

1716
export type FocusableChangeHandler = (isFocusable: boolean) => void;
1817

1918
export const defaultValue: {
2019
navigationActive: boolean;
2120
registerFocusable(focusable: HTMLElement, handler: FocusableChangeHandler): () => void;
22-
resetFocusTarget(): void;
2321
} = {
2422
navigationActive: false,
2523
registerFocusable: () => () => {},
26-
resetFocusTarget: () => {},
2724
};
2825

2926
/**
@@ -102,11 +99,7 @@ export const SingleTabStopNavigationProvider = forwardRef(
10299

103100
// Register a focusable element to allow navigating into it.
104101
// The focusable element tabIndex is only set to 0 if the element matches the focus target.
105-
function registerFocusable(focusableElement: HTMLElement, changeHandler: FocusableChangeHandler) {
106-
// In case the contexts are nested, we must that the components register to all of them,
107-
// so that switching between contexts dynamically is possible.
108-
const parentUnregister = parentContext.registerFocusable(focusableElement, changeHandler);
109-
102+
function registerFocusable(focusableElement: Element, changeHandler: FocusableChangeHandler) {
110103
focusables.current.add(focusableElement);
111104
focusHandlers.current.set(focusableElement, changeHandler);
112105
const isFocusable = !!focusablesState.current.get(focusableElement);
@@ -116,11 +109,7 @@ export const SingleTabStopNavigationProvider = forwardRef(
116109
changeHandler(newIsFocusable);
117110
}
118111
onRegisterFocusable?.(focusableElement);
119-
120-
return () => {
121-
parentUnregister();
122-
unregisterFocusable(focusableElement);
123-
};
112+
return () => unregisterFocusable(focusableElement);
124113
}
125114
function unregisterFocusable(focusableElement: Element) {
126115
focusables.current.delete(focusableElement);
@@ -129,49 +118,32 @@ export const SingleTabStopNavigationProvider = forwardRef(
129118
}
130119

131120
// Update focus target with next single focusable element and notify all registered focusables of a change.
132-
function updateFocusTarget(forceUpdate = false) {
121+
function updateFocusTarget() {
133122
focusTarget.current = getNextFocusTarget();
134123
for (const focusableElement of focusables.current) {
135124
const isFocusable = focusablesState.current.get(focusableElement) ?? false;
136125
const newIsFocusable = focusTarget.current === focusableElement || !!isElementSuppressed?.(focusableElement);
137-
if (newIsFocusable !== isFocusable || forceUpdate) {
126+
if (newIsFocusable !== isFocusable) {
138127
focusablesState.current.set(focusableElement, newIsFocusable);
139128
focusHandlers.current.get(focusableElement)!(newIsFocusable);
140129
}
141130
}
142131
}
143-
function resetFocusTarget() {
144-
updateFocusTarget(true);
145-
}
132+
146133
function getFocusTarget() {
147134
return focusTarget.current;
148135
}
136+
149137
function isRegistered(element: Element) {
150138
return focusables.current.has(element);
151139
}
152-
useImperativeHandle(ref, () => ({ updateFocusTarget, getFocusTarget, isRegistered }));
153140

154-
// Only one STSN context should be active at a time.
155-
// The outer context is preferred over the inners. The components using STSN
156-
// must either work with either outer or inner context, or an explicit switch mechanism
157-
// needs to be implemented (that turns the outer context on and off based on user interaction).
158-
const parentContext = useContext(SingleTabStopNavigationContext);
159-
const value = parentContext.navigationActive
160-
? parentContext
161-
: { navigationActive, registerFocusable, updateFocusTarget, resetFocusTarget };
162-
163-
// When contexts switching occurs, it is essential that the now-active one updates the focus target
164-
// to ensure the tab indices are correctly set.
165-
useEffectOnUpdate(() => {
166-
if (parentContext.navigationActive) {
167-
parentContext.resetFocusTarget();
168-
} else {
169-
resetFocusTarget();
170-
}
171-
// The updateFocusTarget and its dependencies must be pure.
172-
// eslint-disable-next-line react-hooks/exhaustive-deps
173-
}, [parentContext.navigationActive]);
141+
useImperativeHandle(ref, () => ({ updateFocusTarget, getFocusTarget, isRegistered }));
174142

175-
return <SingleTabStopNavigationContext.Provider value={value}>{children}</SingleTabStopNavigationContext.Provider>;
143+
return (
144+
<SingleTabStopNavigationContext.Provider value={{ navigationActive, registerFocusable }}>
145+
{children}
146+
</SingleTabStopNavigationContext.Provider>
147+
);
176148
}
177149
);

0 commit comments

Comments
 (0)