## Fixes #20940 ### Problem The "Move Left" / "Move Right" actions in the table column header menu were unreliable. Clicking them often produced no visible change, or appeared to move the column an inconsistent number of positions. ### Root cause `useMoveRecordField` computed the swap target from **all** record fields (`currentRecordFieldsComponentState`) sorted by position — including hidden and non-readable columns. As a result, a move frequently swapped positions with an *invisible* neighbor, leaving the visible column order unchanged. This was also inconsistent with the drag-and-drop reorder path (`useReorderVisibleRecordFields`), which already operates only on the visible field set, and with the dropdown's own Move enable/disable logic, which is based on `visibleRecordFields`. ### Fix `useMoveRecordField` now sources the neighbor from `visibleRecordFieldsComponentSelector` — the same selector that drives the table display and the Move menu items (`isVisible && isReadable && isActive`, sorted by position). The real `position` values are still swapped, so hidden columns keep their positions and only the visible order changes. ### Tests Added `useMoveRecordField.test.tsx`, which seeds real object metadata with a hidden column interleaved between visible ones (by position) and asserts that the visible selector reorders correctly after a move. The test fails against the previous implementation and passes with this change. ### How to verify 1. Open any table view. 2. Open a column header menu and click "Move Right" / "Move Left". 3. The column now moves reliably by one visible position each click, regardless of hidden columns. --------- Co-authored-by: Harsh Singh <harsh@Harshs-MacBook-Air.local> Co-authored-by: Charles Bochet <charles@twenty.com>
This commit is contained in:
+39
-44
@@ -1,16 +1,16 @@
|
||||
import { useStore } from 'jotai';
|
||||
import { useCallback } from 'react';
|
||||
|
||||
import { useUpdateRecordField } from '@/object-record/record-field/hooks/useUpdateRecordField';
|
||||
import { currentRecordFieldsComponentState } from '@/object-record/record-field/states/currentRecordFieldsComponentState';
|
||||
import { useAtomComponentStateCallbackState } from '@/ui/utilities/state/jotai/hooks/useAtomComponentStateCallbackState';
|
||||
import { visibleRecordFieldsComponentSelector } from '@/object-record/record-field/states/visibleRecordFieldsComponentSelector';
|
||||
import { useAtomComponentSelectorCallbackState } from '@/ui/utilities/state/jotai/hooks/useAtomComponentSelectorCallbackState';
|
||||
import { useSaveCurrentViewFields } from '@/views/hooks/useSaveCurrentViewFields';
|
||||
import { mapRecordFieldToViewField } from '@/views/utils/mapRecordFieldToViewField';
|
||||
import { useCallback } from 'react';
|
||||
import { sortByProperty } from '~/utils/array/sortByProperty';
|
||||
import { useStore } from 'jotai';
|
||||
|
||||
export const useMoveRecordField = (recordTableId?: string) => {
|
||||
const store = useStore();
|
||||
const currentRecordFields = useAtomComponentStateCallbackState(
|
||||
currentRecordFieldsComponentState,
|
||||
const visibleRecordFields = useAtomComponentSelectorCallbackState(
|
||||
visibleRecordFieldsComponentSelector,
|
||||
recordTableId,
|
||||
);
|
||||
|
||||
@@ -26,11 +26,9 @@ export const useMoveRecordField = (recordTableId?: string) => {
|
||||
direction: 'before' | 'after';
|
||||
fieldMetadataItemIdToMove: string;
|
||||
}) => {
|
||||
const sortedRecordFields = store
|
||||
.get(currentRecordFields)
|
||||
.toSorted(sortByProperty('position'));
|
||||
const visibleRecordFieldsValue = store.get(visibleRecordFields);
|
||||
|
||||
const indexOfRecordFieldToMove = sortedRecordFields.findIndex(
|
||||
const indexOfRecordFieldToMove = visibleRecordFieldsValue.findIndex(
|
||||
(recordField) =>
|
||||
recordField.fieldMetadataItemId === fieldMetadataItemIdToMove,
|
||||
);
|
||||
@@ -39,48 +37,45 @@ export const useMoveRecordField = (recordTableId?: string) => {
|
||||
return;
|
||||
}
|
||||
|
||||
const newRecordFields = [...sortedRecordFields];
|
||||
|
||||
const targetArrayIndex =
|
||||
direction === 'before'
|
||||
? indexOfRecordFieldToMove - 1
|
||||
: indexOfRecordFieldToMove + 1;
|
||||
|
||||
const targetArraySize = newRecordFields.length - 1;
|
||||
|
||||
if (
|
||||
indexOfRecordFieldToMove >= 0 &&
|
||||
targetArrayIndex >= 0 &&
|
||||
indexOfRecordFieldToMove <= targetArraySize &&
|
||||
targetArrayIndex <= targetArraySize
|
||||
targetArrayIndex < 0 ||
|
||||
targetArrayIndex > visibleRecordFieldsValue.length - 1
|
||||
) {
|
||||
const currentRecordField = newRecordFields[indexOfRecordFieldToMove];
|
||||
const targetRecordField = newRecordFields[targetArrayIndex];
|
||||
|
||||
const targetRecordFieldNewPosition = currentRecordField.position;
|
||||
const currentRecordFieldNewPosition = targetRecordField.position;
|
||||
|
||||
updateRecordField(targetRecordField.fieldMetadataItemId, {
|
||||
position: targetRecordFieldNewPosition,
|
||||
});
|
||||
|
||||
updateRecordField(currentRecordField.fieldMetadataItemId, {
|
||||
position: currentRecordFieldNewPosition,
|
||||
});
|
||||
|
||||
await saveViewFields([
|
||||
mapRecordFieldToViewField({
|
||||
...targetRecordField,
|
||||
position: targetRecordFieldNewPosition,
|
||||
}),
|
||||
mapRecordFieldToViewField({
|
||||
...currentRecordField,
|
||||
position: currentRecordFieldNewPosition,
|
||||
}),
|
||||
]);
|
||||
return;
|
||||
}
|
||||
|
||||
const currentRecordField =
|
||||
visibleRecordFieldsValue[indexOfRecordFieldToMove];
|
||||
const targetRecordField = visibleRecordFieldsValue[targetArrayIndex];
|
||||
|
||||
const targetRecordFieldNewPosition = currentRecordField.position;
|
||||
const currentRecordFieldNewPosition = targetRecordField.position;
|
||||
|
||||
updateRecordField(targetRecordField.fieldMetadataItemId, {
|
||||
position: targetRecordFieldNewPosition,
|
||||
});
|
||||
|
||||
updateRecordField(currentRecordField.fieldMetadataItemId, {
|
||||
position: currentRecordFieldNewPosition,
|
||||
});
|
||||
|
||||
await saveViewFields([
|
||||
mapRecordFieldToViewField({
|
||||
...targetRecordField,
|
||||
position: targetRecordFieldNewPosition,
|
||||
}),
|
||||
mapRecordFieldToViewField({
|
||||
...currentRecordField,
|
||||
position: currentRecordFieldNewPosition,
|
||||
}),
|
||||
]);
|
||||
},
|
||||
[currentRecordFields, saveViewFields, updateRecordField, store],
|
||||
[visibleRecordFields, saveViewFields, updateRecordField, store],
|
||||
);
|
||||
|
||||
return { moveRecordField };
|
||||
|
||||
Reference in New Issue
Block a user