Skip to content

Commit e159fc3

Browse files
authored
Merge pull request #54682 from FitseTLT/fix-blue-selection-on-page-refresh
Fix - Web - Blue selection border on plan option after a refresh
2 parents f7fff38 + 1c9c6a7 commit e159fc3

File tree

5 files changed

+95
-31
lines changed

5 files changed

+95
-31
lines changed

src/components/SelectionList/BaseSelectionList.tsx

+20-1
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import useSingleExecution from '@hooks/useSingleExecution';
2424
import useStyledSafeAreaInsets from '@hooks/useStyledSafeAreaInsets';
2525
import useThemeStyles from '@hooks/useThemeStyles';
2626
import getSectionsWithIndexOffset from '@libs/getSectionsWithIndexOffset';
27+
import {addKeyDownPressListener, removeKeyDownPressListener} from '@libs/KeyboardShortcut/KeyDownPressListener';
2728
import Log from '@libs/Log';
2829
import variables from '@styles/variables';
2930
import CONST from '@src/CONST';
@@ -129,6 +130,7 @@ function BaseSelectionList<TItem extends ListItem>(
129130
const {translate} = useLocalize();
130131
const listRef = useRef<RNSectionList<TItem, SectionWithIndexOffset<TItem>>>(null);
131132
const innerTextInputRef = useRef<RNTextInput | null>(null);
133+
const hasKeyBeenPressed = useRef(false);
132134
const focusTimeoutRef = useRef<NodeJS.Timeout | null>(null);
133135
const shouldShowSelectAll = !!onSelectAll;
134136
const activeElementRole = useActiveElementRole();
@@ -318,6 +320,16 @@ function BaseSelectionList<TItem extends ListItem>(
318320

319321
const debouncedScrollToIndex = useMemo(() => lodashDebounce(scrollToIndex, CONST.TIMING.LIST_SCROLLING_DEBOUNCE_TIME, {leading: true, trailing: true}), [scrollToIndex]);
320322

323+
const setHasKeyBeenPressed = useCallback(() => {
324+
if (hasKeyBeenPressed.current) {
325+
return;
326+
}
327+
328+
// We need to track whether a key has been pressed to enable focus syncing only if a key has been pressed.
329+
// This is to avoid the default behavior of web showing blue border on click of items after a page refresh.
330+
hasKeyBeenPressed.current = true;
331+
}, []);
332+
321333
// If `initiallyFocusedOptionKey` is not passed, we fall back to `-1`, to avoid showing the highlight on the first member
322334
const [focusedIndex, setFocusedIndex] = useArrowKeyFocusManager({
323335
initialFocusedIndex: flattenedSections.allOptions.findIndex((option) => option.keyForList === initiallyFocusedOptionKey),
@@ -333,9 +345,16 @@ function BaseSelectionList<TItem extends ListItem>(
333345
(shouldDebounceScrolling ? debouncedScrollToIndex : scrollToIndex)(index, true);
334346
}
335347
},
348+
...(!hasKeyBeenPressed.current && {setHasKeyBeenPressed}),
336349
isFocused,
337350
});
338351

352+
useEffect(() => {
353+
addKeyDownPressListener(setHasKeyBeenPressed);
354+
355+
return () => removeKeyDownPressListener(setHasKeyBeenPressed);
356+
}, [setHasKeyBeenPressed]);
357+
339358
const selectedItemIndex = useMemo(
340359
() => (initiallyFocusedOptionKey ? flattenedSections.allOptions.findIndex((option) => option.isSelected) : -1),
341360
[flattenedSections.allOptions, initiallyFocusedOptionKey],
@@ -549,7 +568,7 @@ function BaseSelectionList<TItem extends ListItem>(
549568
shouldIgnoreFocus={shouldIgnoreFocus}
550569
setFocusedIndex={setFocusedIndex}
551570
normalizedIndex={normalizedIndex}
552-
shouldSyncFocus={!isTextInputFocusedRef.current}
571+
shouldSyncFocus={!isTextInputFocusedRef.current && hasKeyBeenPressed.current}
553572
wrapperStyle={listItemWrapperStyle}
554573
titleStyles={listItemTitleStyles}
555574
shouldHighlightSelectedItem={shouldHighlightSelectedItem}

src/hooks/useArrowKeyFocusManager.ts

+6-3
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ type Config = {
1515
allowHorizontalArrowKeys?: boolean;
1616
allowNegativeIndexes?: boolean;
1717
isFocused?: boolean;
18+
setHasKeyBeenPressed?: () => void;
1819
};
1920

2021
type UseArrowKeyFocusManager = [number, (index: number) => void];
@@ -50,6 +51,7 @@ export default function useArrowKeyFocusManager({
5051
allowHorizontalArrowKeys = false,
5152
allowNegativeIndexes = false,
5253
isFocused = true,
54+
setHasKeyBeenPressed,
5355
}: Config): UseArrowKeyFocusManager {
5456
const [focusedIndex, setFocusedIndex] = useState(initialFocusedIndex);
5557
const prevIsFocusedIndex = usePrevious(focusedIndex);
@@ -82,7 +84,7 @@ export default function useArrowKeyFocusManager({
8284
return;
8385
}
8486
const nextIndex = disableCyclicTraversal ? -1 : maxIndex;
85-
87+
setHasKeyBeenPressed?.();
8688
setFocusedIndex((actualIndex) => {
8789
const currentFocusedIndex = actualIndex > 0 ? actualIndex - (itemsPerRow ?? 1) : nextIndex;
8890
let newFocusedIndex = currentFocusedIndex;
@@ -105,14 +107,15 @@ export default function useArrowKeyFocusManager({
105107
}
106108
return newFocusedIndex;
107109
});
108-
}, [maxIndex, isFocused, disableCyclicTraversal, itemsPerRow, disabledIndexes, allowNegativeIndexes]);
110+
}, [maxIndex, isFocused, disableCyclicTraversal, itemsPerRow, disabledIndexes, allowNegativeIndexes, setHasKeyBeenPressed]);
109111

110112
useKeyboardShortcut(CONST.KEYBOARD_SHORTCUTS.ARROW_UP, arrowUpCallback, arrowConfig);
111113

112114
const arrowDownCallback = useCallback(() => {
113115
if (maxIndex < 0 || !isFocused) {
114116
return;
115117
}
118+
setHasKeyBeenPressed?.();
116119

117120
const nextIndex = disableCyclicTraversal ? maxIndex : 0;
118121

@@ -150,7 +153,7 @@ export default function useArrowKeyFocusManager({
150153
}
151154
return newFocusedIndex;
152155
});
153-
}, [disableCyclicTraversal, disabledIndexes, isFocused, itemsPerRow, maxIndex]);
156+
}, [disableCyclicTraversal, disabledIndexes, isFocused, itemsPerRow, maxIndex, setHasKeyBeenPressed]);
154157

155158
useKeyboardShortcut(CONST.KEYBOARD_SHORTCUTS.ARROW_DOWN, arrowDownCallback, arrowConfig);
156159

src/hooks/useSyncFocus/index.ts

+2-27
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,3 @@
1-
import {useContext, useLayoutEffect} from 'react';
2-
import type {RefObject} from 'react';
3-
import type {View} from 'react-native';
4-
import {ScreenWrapperStatusContext} from '@components/ScreenWrapper';
1+
import useSyncFocusImplementation from './useSyncFocusImplementation';
52

6-
/**
7-
* Custom React hook created to handle sync of focus on an element when the user navigates through the app with keyboard.
8-
* When the user navigates through the app using the arrows and then the tab button, the focus on the element and the native focus of the browser differs.
9-
* To maintain consistency when an element is focused in the app, the focus() method is additionally called on the focused element to eliminate the difference between native browser focus and application focus.
10-
*/
11-
const useSyncFocus = (ref: RefObject<View | HTMLElement>, isFocused: boolean, shouldSyncFocus = true) => {
12-
// this hook can be used outside ScreenWrapperStatusContext (eg. in Popovers). So we to check if the context is present.
13-
// If we are outside context we don't have to look at transition status
14-
const contextValue = useContext(ScreenWrapperStatusContext);
15-
16-
const didScreenTransitionEnd = contextValue ? contextValue.didScreenTransitionEnd : true;
17-
18-
useLayoutEffect(() => {
19-
if (!isFocused || !shouldSyncFocus || !didScreenTransitionEnd) {
20-
return;
21-
}
22-
23-
ref.current?.focus({preventScroll: true});
24-
// eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
25-
}, [didScreenTransitionEnd, isFocused, ref]);
26-
};
27-
28-
export default useSyncFocus;
3+
export default useSyncFocusImplementation;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
import {useContext, useLayoutEffect} from 'react';
2+
import type {RefObject} from 'react';
3+
import type {View} from 'react-native';
4+
import {ScreenWrapperStatusContext} from '@components/ScreenWrapper';
5+
6+
/**
7+
* Custom React hook created to handle sync of focus on an element when the user navigates through the app with keyboard.
8+
* When the user navigates through the app using the arrows and then the tab button, the focus on the element and the native focus of the browser differs.
9+
* To maintain consistency when an element is focused in the app, the focus() method is additionally called on the focused element to eliminate the difference between native browser focus and application focus.
10+
*/
11+
const useSyncFocusImplementation = (ref: RefObject<View | HTMLElement>, isFocused: boolean, shouldSyncFocus = true) => {
12+
// this hook can be used outside ScreenWrapperStatusContext (eg. in Popovers). So we to check if the context is present.
13+
// If we are outside context we don't have to look at transition status
14+
const contextValue = useContext(ScreenWrapperStatusContext);
15+
16+
const didScreenTransitionEnd = contextValue ? contextValue.didScreenTransitionEnd : true;
17+
18+
useLayoutEffect(() => {
19+
if (!isFocused || !shouldSyncFocus || !didScreenTransitionEnd) {
20+
return;
21+
}
22+
23+
ref.current?.focus({preventScroll: true});
24+
// eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
25+
}, [didScreenTransitionEnd, isFocused, ref]);
26+
};
27+
28+
export default useSyncFocusImplementation;

tests/unit/useSyncFocusTest.ts

+39
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
import {renderHook} from '@testing-library/react-native';
2+
import type {RefObject} from 'react';
3+
import type {View} from 'react-native';
4+
import useSyncFocus from '@hooks/useSyncFocus/useSyncFocusImplementation';
5+
6+
describe('useSyncFocus', () => {
7+
it('useSyncFocus should only focus if shouldSyncFocus is true', () => {
8+
const refMock = {current: {focus: jest.fn()}};
9+
10+
// When useSyncFocus is rendered initially while shouldSyncFocus is false.
11+
const {rerender} = renderHook(
12+
({
13+
ref = refMock,
14+
isFocused = true,
15+
shouldSyncFocus = false,
16+
}: {
17+
isFocused?: boolean;
18+
shouldSyncFocus?: boolean;
19+
ref?: {
20+
current: {
21+
focus: jest.Mock;
22+
};
23+
};
24+
}) => useSyncFocus(ref as unknown as RefObject<View>, isFocused, shouldSyncFocus),
25+
{initialProps: {}},
26+
);
27+
// Then the ref focus will not be called.
28+
expect(refMock.current.focus).not.toHaveBeenCalled();
29+
30+
rerender({isFocused: false});
31+
expect(refMock.current.focus).not.toHaveBeenCalled();
32+
33+
// When shouldSyncFocus and isFocused are true
34+
rerender({isFocused: true, shouldSyncFocus: true});
35+
36+
// Then the ref focus will be called.
37+
expect(refMock.current.focus).toHaveBeenCalled();
38+
});
39+
});

0 commit comments

Comments
 (0)