Skip to content

Commit 7fb72e7

Browse files
authored
fix(react-tabster): improve support for concurrent mode (#34113)
1 parent 4d83497 commit 7fb72e7

13 files changed

+194
-86
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "patch",
3+
"comment": "fix: improve support for concurrent mode",
4+
"packageName": "@fluentui/react-tabster",
5+
"email": "[email protected]",
6+
"dependentChangeType": "patch"
7+
}

packages/react-components/react-tabster/src/hooks/useActivateModal.ts

+4-5
Original file line numberDiff line numberDiff line change
@@ -7,21 +7,20 @@ import { useTabster } from './useTabster';
77
* Returns a function that activates a modal by element from the modal or modal container.
88
*/
99
export function useActivateModal(): (elementFromModal: HTMLElement | undefined) => void {
10-
const tabster = useTabster();
11-
const modalizerAPI = tabster ? getModalizer(tabster) : undefined;
12-
const [setActivateModalTimeout] = useTimeout();
10+
const modalizerRefAPI = useTabster(getModalizer);
1311

12+
const [setActivateModalTimeout] = useTimeout();
1413
const activateModal = React.useCallback(
1514
(elementFromModal: HTMLElement | undefined) => {
1615
// We call the actual activation function on the next tick, because with the typical use case,
1716
// the hook will be called on the same tick when other Tabster attributes are being applied,
1817
// and on the current tick the element has just received the attributes, but Tabster has not
1918
// instantiated the Modalizer yet.
2019
setActivateModalTimeout(() => {
21-
modalizerAPI?.activate(elementFromModal);
20+
modalizerRefAPI.current?.activate(elementFromModal);
2221
}, 0);
2322
},
24-
[modalizerAPI, setActivateModalTimeout],
23+
[modalizerRefAPI, setActivateModalTimeout],
2524
);
2625

2726
return activateModal;

packages/react-components/react-tabster/src/hooks/useArrowNavigationGroup.ts

+1-4
Original file line numberDiff line numberDiff line change
@@ -49,11 +49,8 @@ export const useArrowNavigationGroup = (options: UseArrowNavigationGroupOptions
4949
// eslint-disable-next-line @typescript-eslint/naming-convention
5050
unstable_hasDefault,
5151
} = options;
52-
const tabster = useTabster();
5352

54-
if (tabster) {
55-
getMover(tabster);
56-
}
53+
useTabster(getMover);
5754

5855
return useTabsterAttributes({
5956
mover: {

packages/react-components/react-tabster/src/hooks/useFocusFinders.ts

+13-13
Original file line numberDiff line numberDiff line change
@@ -7,50 +7,50 @@ import { useTabster } from './useTabster';
77
* Returns a set of helper functions that will traverse focusable elements in the context of a root DOM element
88
*/
99
export const useFocusFinders = () => {
10-
const tabster = useTabster();
10+
const tabsterRef = useTabster();
1111
const { targetDocument } = useFluent();
1212

1313
// Narrow props for now and let need dictate additional props in the future
1414
const findAllFocusable = React.useCallback(
1515
(container: HTMLElement, acceptCondition?: (el: HTMLElement) => boolean) =>
16-
tabster?.focusable.findAll({ container, acceptCondition }) || [],
17-
[tabster],
16+
tabsterRef.current?.focusable.findAll({ container, acceptCondition }) || [],
17+
[tabsterRef],
1818
);
1919

2020
const findFirstFocusable = React.useCallback(
21-
(container: HTMLElement) => tabster?.focusable.findFirst({ container }),
22-
[tabster],
21+
(container: HTMLElement) => tabsterRef.current?.focusable.findFirst({ container }),
22+
[tabsterRef],
2323
);
2424

2525
const findLastFocusable = React.useCallback(
26-
(container: HTMLElement) => tabster?.focusable.findLast({ container }),
27-
[tabster],
26+
(container: HTMLElement) => tabsterRef.current?.focusable.findLast({ container }),
27+
[tabsterRef],
2828
);
2929

3030
const findNextFocusable = React.useCallback(
3131
(currentElement: HTMLElement, options: Pick<Partial<TabsterTypes.FindNextProps>, 'container'> = {}) => {
32-
if (!tabster || !targetDocument) {
32+
if (!tabsterRef.current || !targetDocument) {
3333
return null;
3434
}
3535

3636
const { container = targetDocument.body } = options;
3737

38-
return tabster.focusable.findNext({ currentElement, container });
38+
return tabsterRef.current.focusable.findNext({ currentElement, container });
3939
},
40-
[tabster, targetDocument],
40+
[tabsterRef, targetDocument],
4141
);
4242

4343
const findPrevFocusable = React.useCallback(
4444
(currentElement: HTMLElement, options: Pick<Partial<TabsterTypes.FindNextProps>, 'container'> = {}) => {
45-
if (!tabster || !targetDocument) {
45+
if (!tabsterRef.current || !targetDocument) {
4646
return null;
4747
}
4848

4949
const { container = targetDocument.body } = options;
5050

51-
return tabster.focusable.findPrev({ currentElement, container });
51+
return tabsterRef.current.focusable.findPrev({ currentElement, container });
5252
},
53-
[tabster, targetDocument],
53+
[tabsterRef, targetDocument],
5454
);
5555

5656
return {

packages/react-components/react-tabster/src/hooks/useFocusObserved.ts

+13-9
Original file line numberDiff line numberDiff line change
@@ -10,24 +10,28 @@ interface UseFocusObservedOptions {
1010
}
1111

1212
/**
13-
*
1413
* @param name - The observed element to focus
15-
* @returns Function that will focus the
14+
* @param options - Options for the focus observed
15+
*
16+
* @returns Function that will focus an element
1617
*/
1718
export function useFocusObserved(
1819
name: string,
1920
options: UseFocusObservedOptions = {},
2021
): () => TabsterTypes.ObservedElementAsyncRequest<boolean> {
2122
const { timeout = 1000 } = options;
22-
const tabster = useTabster();
23-
24-
const observedAPI = tabster ? getObservedElement(tabster) : null;
23+
const observedAPIRef = useTabster(getObservedElement);
2524

2625
return React.useCallback(() => {
27-
if (observedAPI) {
28-
return observedAPI.requestFocus(name, timeout);
26+
const observerAPI = observedAPIRef.current;
27+
28+
if (observerAPI) {
29+
return observerAPI.requestFocus(name, timeout);
2930
}
3031

31-
return { result: Promise.resolve(false), cancel: () => null };
32-
}, [observedAPI, name, timeout]);
32+
return {
33+
result: Promise.resolve(false),
34+
cancel: () => null,
35+
};
36+
}, [observedAPIRef, name, timeout]);
3337
}

packages/react-components/react-tabster/src/hooks/useFocusableGroup.ts

+1-5
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,7 @@ export interface UseFocusableGroupOptions {
1919
* @param options - Options to configure keyboard navigation
2020
*/
2121
export const useFocusableGroup = (options?: UseFocusableGroupOptions): Types.TabsterDOMAttribute => {
22-
const tabster = useTabster();
23-
24-
if (tabster) {
25-
getGroupper(tabster);
26-
}
22+
useTabster(getGroupper);
2723

2824
return useTabsterAttributes({
2925
groupper: {
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
1-
import type { Types as TabsterTypes } from 'tabster';
2-
3-
import { useTabster } from './useTabster';
1+
import { type Types as TabsterTypes, disposeTabster } from 'tabster';
42
import * as React from 'react';
3+
import { useFluent_unstable as useFluent } from '@fluentui/react-shared-contexts';
4+
import { useEventCallback } from '@fluentui/react-utilities';
5+
6+
import { createTabsterWithConfig } from './useTabster';
57

68
/**
79
* Subscribes to the tabster focused element. Calls the callback when the focused element changes.
@@ -10,11 +12,19 @@ import * as React from 'react';
1012
export function useFocusedElementChange(
1113
callback: TabsterTypes.SubscribableCallback<HTMLElement | undefined, TabsterTypes.FocusedElementDetail>,
1214
) {
13-
const tabster = useTabster();
15+
const { targetDocument } = useFluent();
16+
const listener = useEventCallback(callback);
17+
1418
React.useEffect(() => {
19+
const tabster = createTabsterWithConfig(targetDocument);
20+
1521
if (tabster) {
16-
tabster.focusedElement.subscribe(callback);
17-
return () => tabster.focusedElement.unsubscribe(callback);
22+
tabster.focusedElement.subscribe(listener);
23+
24+
return () => {
25+
tabster.focusedElement.unsubscribe(listener);
26+
disposeTabster(tabster);
27+
};
1828
}
19-
}, [tabster, callback]);
29+
}, [listener, targetDocument]);
2030
}

packages/react-components/react-tabster/src/hooks/useModalAttributes.ts

+7-5
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,11 @@ const tabsterAccessibleCheck: TabsterTypes.ModalizerElementAccessibleCheck = ele
4646
return element.hasAttribute(DangerousNeverHiddenAttribute);
4747
};
4848

49+
function initTabsterModules(tabster: TabsterTypes.TabsterCore) {
50+
getModalizer(tabster, undefined, tabsterAccessibleCheck);
51+
getRestorer(tabster);
52+
}
53+
4954
/**
5055
* Applies modal dialog behaviour through DOM attributes
5156
* Modal element will focus trap and hide other content on the page
@@ -57,12 +62,9 @@ export const useModalAttributes = (
5762
options: UseModalAttributesOptions = {},
5863
): { modalAttributes: TabsterTypes.TabsterDOMAttribute; triggerAttributes: TabsterTypes.TabsterDOMAttribute } => {
5964
const { trapFocus, alwaysFocusable, legacyTrapFocus } = options;
60-
const tabster = useTabster();
65+
6166
// Initializes the modalizer and restorer APIs
62-
if (tabster) {
63-
getModalizer(tabster, undefined, tabsterAccessibleCheck);
64-
getRestorer(tabster);
65-
}
67+
useTabster(initTabsterModules);
6668

6769
const id = useId('modal-', options.id);
6870
const modalAttributes = useTabsterAttributes({
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,10 @@
1-
import { useTabster } from './useTabster';
21
import { getObservedElement, Types as TabsterTypes } from 'tabster';
2+
3+
import { useTabster } from './useTabster';
34
import { useTabsterAttributes } from './useTabsterAttributes';
45

56
export function useObservedElement(name: string | string[]): TabsterTypes.TabsterDOMAttribute {
6-
const tabster = useTabster();
7-
if (tabster) {
8-
getObservedElement(tabster);
9-
}
7+
useTabster(getObservedElement);
108

119
return useTabsterAttributes({ observed: { names: Array.isArray(name) ? name : [name] } });
1210
}

packages/react-components/react-tabster/src/hooks/useRestoreFocus.ts

+2-8
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,8 @@ import { useTabster } from './useTabster';
66
* @returns Attribute to apply to the target element where focus is restored
77
*/
88
export function useRestoreFocusTarget(): TabsterTypes.TabsterDOMAttribute {
9-
const tabster = useTabster();
109
// Initializes the restorer API
11-
if (tabster) {
12-
getRestorer(tabster);
13-
}
10+
useTabster(getRestorer);
1411

1512
return getTabsterAttribute({ restorer: { type: RestorerTypes.Target } });
1613
}
@@ -20,11 +17,8 @@ export function useRestoreFocusTarget(): TabsterTypes.TabsterDOMAttribute {
2017
* @returns Attribute to apply to the element that might lose focus
2118
*/
2219
export function useRestoreFocusSource(): TabsterTypes.TabsterDOMAttribute {
23-
const tabster = useTabster();
2420
// Initializes the restorer API
25-
if (tabster) {
26-
getRestorer(tabster);
27-
}
21+
useTabster(getRestorer);
2822

2923
return getTabsterAttribute({ restorer: { type: RestorerTypes.Source } });
3024
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
import { createTabster, disposeTabster } from 'tabster';
2+
import { renderHook } from '@testing-library/react-hooks';
3+
import { useTabster } from './useTabster';
4+
5+
jest.mock('tabster', () => ({
6+
createTabster: jest.fn(() => 'mock-tabster-instance'),
7+
disposeTabster: jest.fn(),
8+
}));
9+
10+
const createTabsterMock = createTabster as jest.Mock;
11+
const disposeTabsterMock = disposeTabster as jest.Mock;
12+
13+
describe('useTabster', () => {
14+
it('returns a ref to the tabster instance', () => {
15+
const { result } = renderHook(() => useTabster());
16+
17+
expect(createTabsterMock).toHaveBeenCalledTimes(1);
18+
expect(result.current).toMatchObject({
19+
current: 'mock-tabster-instance',
20+
});
21+
});
22+
23+
it('uses the provided factory function to transform the tabster instance', () => {
24+
const factoryMock = jest.fn(tabster => ({ customProperty: 'value', tabster }));
25+
const { result } = renderHook(() => useTabster(factoryMock));
26+
27+
expect(factoryMock).toHaveBeenCalledWith('mock-tabster-instance');
28+
expect(result.current).toMatchObject({
29+
current: { customProperty: 'value', tabster: 'mock-tabster-instance' },
30+
});
31+
});
32+
33+
it('disposes tabster when the component unmounts', () => {
34+
const { unmount } = renderHook(() => useTabster());
35+
36+
unmount();
37+
expect(disposeTabsterMock).toHaveBeenCalledWith('mock-tabster-instance');
38+
});
39+
40+
describe('environment checks', () => {
41+
let originalNodeEnv: string;
42+
43+
beforeEach(() => {
44+
originalNodeEnv = process.env.NODE_ENV ?? '';
45+
});
46+
47+
afterEach(() => {
48+
process.env.NODE_ENV = originalNodeEnv;
49+
});
50+
51+
it('throws an error in development if factory function changes', () => {
52+
process.env.NODE_ENV = 'development';
53+
54+
const mockFactory1 = jest.fn(tabster => tabster);
55+
const mockFactory2 = jest.fn(tabster => tabster);
56+
57+
const { result, rerender } = renderHook(({ factory }) => useTabster(factory), {
58+
initialProps: { factory: mockFactory1 },
59+
});
60+
61+
rerender({ factory: mockFactory2 });
62+
63+
expect(result.error).toMatchInlineSnapshot(`
64+
[Error: @fluentui/react-tabster:
65+
The factory function passed to useTabster has changed. This should not ever happen.]
66+
`);
67+
});
68+
});
69+
});

0 commit comments

Comments
 (0)