Fix selectable list arrow focus (#21679)
## Summary Tested on all select fields one by one - Keep searchable selectable-list inputs focused while ArrowUp/ArrowDown moves the selected item. - Remove the old global "grid focused" mode and blur/refocus recovery path. - Scroll the selected item into view with `block: 'nearest'`, which restores keyboard scrolling in long relation pickers without forcing the row to the top. - Add focused regressions for command-menu input focus and selected-item scrolling. ## Root Cause `SelectableList` hotkeys blurred the active input before arrow navigation and stored a global grid-focused state. That let ArrowDown move selection, but focus could fall back to the underlying page/table instead of remaining in the command menu input. ## Recording ### Before https://github.com/user-attachments/assets/a802cbc3-4cfd-4466-bc22-274935a77715 ### After https://github.com/user-attachments/assets/9c315d2e-dcb1-424d-80c9-a5942eb1b6bd ## QA Note I checked all inputs one by one with a 2h30 agent in goal mode. <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/21679?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. -->
This commit is contained in:
committed by
GitHub
parent
257f130fff
commit
079040f1c0
+1
-8
@@ -1,18 +1,11 @@
|
||||
import { useStore } from 'jotai';
|
||||
import { useEffect, useRef } from 'react';
|
||||
import { isDefined } from 'twenty-shared/utils';
|
||||
|
||||
import { isSelectableListGridFocusedState } from '@/ui/layout/selectable-list/states/isSelectableListGridFocusedState';
|
||||
|
||||
export const useInputFocusWithoutScrollOnMount = () => {
|
||||
const inputRef = useRef<HTMLInputElement>(null);
|
||||
const store = useStore();
|
||||
|
||||
useEffect(() => {
|
||||
if (
|
||||
isDefined(inputRef.current) &&
|
||||
!store.get(isSelectableListGridFocusedState.atom)
|
||||
) {
|
||||
if (isDefined(inputRef.current)) {
|
||||
inputRef.current.focus({ preventScroll: true });
|
||||
}
|
||||
});
|
||||
|
||||
-9
@@ -1,10 +1,8 @@
|
||||
import { useStore } from 'jotai';
|
||||
import { type ReactNode, useEffect } from 'react';
|
||||
|
||||
import { useSelectableListHotKeys } from '@/ui/layout/selectable-list/hooks/internal/useSelectableListHotKeys';
|
||||
import { SelectableListComponentInstanceContext } from '@/ui/layout/selectable-list/states/contexts/SelectableListComponentInstanceContext';
|
||||
import { SelectableListContextProvider } from '@/ui/layout/selectable-list/states/contexts/SelectableListContext';
|
||||
import { isSelectableListGridFocusedState } from '@/ui/layout/selectable-list/states/isSelectableListGridFocusedState';
|
||||
import { selectableItemIdsComponentState } from '@/ui/layout/selectable-list/states/selectableItemIdsComponentState';
|
||||
import { useSetAtomComponentState } from '@/ui/utilities/state/jotai/hooks/useSetAtomComponentState';
|
||||
import { isDefined } from 'twenty-shared/utils';
|
||||
@@ -29,7 +27,6 @@ export const SelectableList = ({
|
||||
}: SelectableListProps) => {
|
||||
useSelectableListHotKeys(selectableListInstanceId, focusId, onSelect);
|
||||
|
||||
const store = useStore();
|
||||
const setSelectableItemIds = useSetAtomComponentState(
|
||||
selectableItemIdsComponentState,
|
||||
selectableListInstanceId,
|
||||
@@ -51,12 +48,6 @@ export const SelectableList = ({
|
||||
}
|
||||
}, [selectableItemIdArray, selectableItemIdMatrix, setSelectableItemIds]);
|
||||
|
||||
useEffect(() => {
|
||||
return () => {
|
||||
store.set(isSelectableListGridFocusedState.atom, false);
|
||||
};
|
||||
}, [store]);
|
||||
|
||||
return (
|
||||
<SelectableListComponentInstanceContext.Provider
|
||||
value={{
|
||||
|
||||
+2
-10
@@ -39,17 +39,9 @@ export const SelectableListItem = ({
|
||||
return;
|
||||
}
|
||||
|
||||
const scrollContainer = listItemRef.current.closest(
|
||||
'[id^="scroll-wrapper-"]',
|
||||
) as HTMLElement | null;
|
||||
|
||||
if (isDefined(scrollContainer) && scrollContainer.scrollTop === 0) {
|
||||
return;
|
||||
}
|
||||
|
||||
listItemRef.current.scrollIntoView({
|
||||
listItemRef.current.scrollIntoView?.({
|
||||
behavior: 'auto',
|
||||
block: 'start',
|
||||
block: 'nearest',
|
||||
});
|
||||
}, [isSelectedItemId]);
|
||||
|
||||
|
||||
+78
@@ -0,0 +1,78 @@
|
||||
import { render, waitFor } from '@testing-library/react';
|
||||
import { createStore, Provider as JotaiProvider } from 'jotai';
|
||||
|
||||
import { SelectableList } from '@/ui/layout/selectable-list/components/SelectableList';
|
||||
import { SelectableListItem } from '@/ui/layout/selectable-list/components/SelectableListItem';
|
||||
import { isSelectedItemIdComponentFamilyState } from '@/ui/layout/selectable-list/states/isSelectedItemIdComponentFamilyState';
|
||||
|
||||
const selectableListInstanceId = 'test-selectable-list';
|
||||
|
||||
const originalScrollIntoView = HTMLElement.prototype.scrollIntoView;
|
||||
|
||||
describe('SelectableListItem', () => {
|
||||
const scrollIntoViewMock = jest.fn();
|
||||
|
||||
beforeEach(() => {
|
||||
scrollIntoViewMock.mockClear();
|
||||
|
||||
Object.defineProperty(HTMLElement.prototype, 'scrollIntoView', {
|
||||
configurable: true,
|
||||
value: scrollIntoViewMock,
|
||||
});
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
if (typeof originalScrollIntoView === 'function') {
|
||||
Object.defineProperty(HTMLElement.prototype, 'scrollIntoView', {
|
||||
configurable: true,
|
||||
value: originalScrollIntoView,
|
||||
});
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
delete (
|
||||
HTMLElement.prototype as {
|
||||
scrollIntoView?: HTMLElement['scrollIntoView'];
|
||||
}
|
||||
).scrollIntoView;
|
||||
});
|
||||
|
||||
it('scrolls the selected item into view even when the scroll wrapper is at the top', async () => {
|
||||
const store = createStore();
|
||||
|
||||
store.set(
|
||||
isSelectedItemIdComponentFamilyState.atomFamily({
|
||||
instanceId: selectableListInstanceId,
|
||||
familyKey: 'second-item',
|
||||
}),
|
||||
true,
|
||||
);
|
||||
|
||||
render(
|
||||
<JotaiProvider store={store}>
|
||||
<div id="scroll-wrapper-test">
|
||||
<SelectableList
|
||||
selectableListInstanceId={selectableListInstanceId}
|
||||
selectableItemIdArray={['first-item', 'second-item']}
|
||||
focusId="test-focus-id"
|
||||
>
|
||||
<SelectableListItem itemId="first-item">
|
||||
First item
|
||||
</SelectableListItem>
|
||||
<SelectableListItem itemId="second-item">
|
||||
Second item
|
||||
</SelectableListItem>
|
||||
</SelectableList>
|
||||
</div>
|
||||
</JotaiProvider>,
|
||||
);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(scrollIntoViewMock).toHaveBeenCalledWith({
|
||||
behavior: 'auto',
|
||||
block: 'nearest',
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
+6
-78
@@ -1,9 +1,8 @@
|
||||
import { isNonEmptyString } from '@sniptt/guards';
|
||||
import { useStore } from 'jotai';
|
||||
import { useCallback, useRef } from 'react';
|
||||
import { useCallback } from 'react';
|
||||
import { Key } from 'ts-key-enum';
|
||||
|
||||
import { isSelectableListGridFocusedState } from '@/ui/layout/selectable-list/states/isSelectableListGridFocusedState';
|
||||
import { isSelectedItemIdComponentFamilyState } from '@/ui/layout/selectable-list/states/isSelectedItemIdComponentFamilyState';
|
||||
import { selectableItemIdsComponentState } from '@/ui/layout/selectable-list/states/selectableItemIdsComponentState';
|
||||
import { selectedItemIdComponentState } from '@/ui/layout/selectable-list/states/selectedItemIdComponentState';
|
||||
@@ -16,41 +15,8 @@ export const useSelectableListHotKeys = (
|
||||
focusId: string,
|
||||
onSelect?: (itemId: string) => void,
|
||||
) => {
|
||||
// oxlint-disable-next-line twenty/no-state-useref
|
||||
const lastBlurredInputRef = useRef<HTMLInputElement | null>(null);
|
||||
|
||||
const store = useStore();
|
||||
|
||||
const blurActiveInputIfNeeded = () => {
|
||||
if (document.activeElement instanceof HTMLInputElement) {
|
||||
lastBlurredInputRef.current = document.activeElement;
|
||||
store.set(isSelectableListGridFocusedState.atom, true);
|
||||
document.activeElement.blur();
|
||||
}
|
||||
};
|
||||
|
||||
const refocusBlurredInput = () => {
|
||||
if (!lastBlurredInputRef.current) {
|
||||
return;
|
||||
}
|
||||
store.set(isSelectableListGridFocusedState.atom, false);
|
||||
lastBlurredInputRef.current.focus();
|
||||
lastBlurredInputRef.current = null;
|
||||
};
|
||||
|
||||
const clearSelection = (selectedItemId: string | null) => {
|
||||
if (isNonEmptyString(selectedItemId)) {
|
||||
store.set(selectedItemIdComponentState.atomFamily({ instanceId }), null);
|
||||
store.set(
|
||||
isSelectedItemIdComponentFamilyState.atomFamily({
|
||||
instanceId,
|
||||
familyKey: selectedItemId,
|
||||
}),
|
||||
false,
|
||||
);
|
||||
}
|
||||
};
|
||||
|
||||
const findPosition = (
|
||||
selectableItemIds: string[][],
|
||||
selectedItemId?: string | null,
|
||||
@@ -173,57 +139,19 @@ export const useSelectableListHotKeys = (
|
||||
useHotkeysOnFocusedElement({
|
||||
keys: Key.ArrowUp,
|
||||
callback: () => {
|
||||
blurActiveInputIfNeeded();
|
||||
|
||||
const selectedItemId = store.get(
|
||||
selectedItemIdComponentState.atomFamily({ instanceId }),
|
||||
);
|
||||
const selectableItemIds = store.get(
|
||||
selectableItemIdsComponentState.atomFamily({ instanceId }),
|
||||
);
|
||||
const position = findPosition(selectableItemIds, selectedItemId);
|
||||
const isAtTop = position !== undefined && position.row === 0;
|
||||
|
||||
if (!isAtTop || !lastBlurredInputRef.current) {
|
||||
handleSelect('up');
|
||||
return;
|
||||
}
|
||||
|
||||
clearSelection(selectedItemId);
|
||||
refocusBlurredInput();
|
||||
},
|
||||
focusId,
|
||||
dependencies: [handleSelect, store, instanceId],
|
||||
});
|
||||
|
||||
useHotkeysOnFocusedElement({
|
||||
keys: Key.ArrowDown,
|
||||
callback: () => {
|
||||
blurActiveInputIfNeeded();
|
||||
handleSelect('down');
|
||||
handleSelect('up');
|
||||
},
|
||||
focusId,
|
||||
dependencies: [handleSelect],
|
||||
});
|
||||
|
||||
useHotkeysOnFocusedElement({
|
||||
keys: '*',
|
||||
callback: (keyboardEvent) => {
|
||||
if (keyboardEvent.key.length !== 1) {
|
||||
return;
|
||||
}
|
||||
if (
|
||||
keyboardEvent.metaKey ||
|
||||
keyboardEvent.ctrlKey ||
|
||||
keyboardEvent.altKey
|
||||
) {
|
||||
return;
|
||||
}
|
||||
refocusBlurredInput();
|
||||
keys: Key.ArrowDown,
|
||||
callback: () => {
|
||||
handleSelect('down');
|
||||
},
|
||||
focusId,
|
||||
dependencies: [],
|
||||
options: { enableOnFormTags: false, preventDefault: false },
|
||||
dependencies: [handleSelect],
|
||||
});
|
||||
|
||||
useHotkeysOnFocusedElement({
|
||||
|
||||
-6
@@ -1,6 +0,0 @@
|
||||
import { createAtomState } from '@/ui/utilities/state/jotai/utils/createAtomState';
|
||||
|
||||
export const isSelectableListGridFocusedState = createAtomState<boolean>({
|
||||
key: 'isSelectableListGridFocusedState',
|
||||
defaultValue: false,
|
||||
});
|
||||
Reference in New Issue
Block a user