fix: fix field select options positions after option removal (#5350)

- Adds an util `toSpliced`. We cannot used the native Javascript
`Array.prototype.toSpliced` method as Chromatic servers don't support
it.
- Makes sure Select field options have sequential positions after
removing an option (form validation schema checks that positions are
sequential and considers options invalid otherwise).
This commit is contained in:
Thaïs
2024-05-10 18:31:22 +02:00
committed by GitHub
parent 72521d5554
commit 0fb416d17c
5 changed files with 99 additions and 16 deletions

View File

@@ -19,6 +19,7 @@ import { DraggableItem } from '@/ui/layout/draggable-list/components/DraggableIt
import { DraggableList } from '@/ui/layout/draggable-list/components/DraggableList';
import { FieldMetadataType } from '~/generated-metadata/graphql';
import { moveArrayItem } from '~/utils/array/moveArrayItem';
import { toSpliced } from '~/utils/array/toSpliced';
import { applySimpleQuotesToString } from '~/utils/string/applySimpleQuotesToString';
import { simpleQuotesStringSchema } from '~/utils/validation-schemas/simpleQuotesStringSchema';
@@ -198,8 +199,12 @@ export const SettingsDataModelFieldSelectForm = ({
key={option.id}
option={option}
onChange={(nextOption) => {
const nextOptions = [...options];
nextOptions.splice(index, 1, nextOption);
const nextOptions = toSpliced(
options,
index,
1,
nextOption,
);
onChange(nextOptions);
// Update option value in defaultValue if value has changed
@@ -211,15 +216,17 @@ export const SettingsDataModelFieldSelectForm = ({
handleSetOptionAsDefault(nextOption.value);
}
}}
onRemove={
options.length > 1
? () => {
const nextOptions = [...options];
nextOptions.splice(index, 1);
onChange(nextOptions);
}
: undefined
}
onRemove={() => {
const nextOptions = toSpliced(
options,
index,
1,
).map((option, nextOptionIndex) => ({
...option,
position: nextOptionIndex,
}));
onChange(nextOptions);
}}
isDefault={isOptionDefaultValue(option.value)}
onSetAsDefault={() =>
handleSetOptionAsDefault(option.value)

View File

@@ -8,6 +8,7 @@ import {
} from '@/ui/utilities/pointer-event/hooks/useListenClickOutsideV2';
import { ClickOutsideListenerCallback } from '@/ui/utilities/pointer-event/types/ClickOutsideListenerCallback';
import { getScopeIdFromComponentId } from '@/ui/utilities/recoil-scope/utils/getScopeIdFromComponentId';
import { toSpliced } from '~/utils/array/toSpliced';
import { isDefined } from '~/utils/isDefined';
export const useClickOutsideListener = (componentId: string) => {
@@ -117,8 +118,11 @@ export const useClickOutsideListener = (componentId: string) => {
const callbackToUnsubscribeIsFound = indexOfCallbackToUnsubscribe > -1;
if (callbackToUnsubscribeIsFound) {
const newCallbacksWithoutCallbackToUnsubscribe =
existingCallbacks.toSpliced(indexOfCallbackToUnsubscribe, 1);
const newCallbacksWithoutCallbackToUnsubscribe = toSpliced(
existingCallbacks,
indexOfCallbackToUnsubscribe,
1,
);
set(
getClickOutsideListenerCallbacksState,

View File

@@ -0,0 +1,37 @@
import { toSpliced } from '../toSpliced';
describe('toSpliced', () => {
it('removes elements from the array starting at the given index', () => {
// Given
const array = ['a', 'b', 'c', 'd', 'e'];
// When
const result = toSpliced(array, 2, 2);
// Then
expect(result).toEqual(['a', 'b', 'e']);
});
it('replaces elements in the array starting at the given index', () => {
// Given
const array = ['a', 'b', 'c', 'd', 'e'];
// When
const result = toSpliced(array, 1, 2, 'x', 'y');
// Then
expect(result).toEqual(['a', 'x', 'y', 'd', 'e']);
});
it('returns a new array without modifying the original array', () => {
// Given
const array = ['a', 'b', 'c'];
// When
const result = toSpliced(array, 0, 1);
// Then
expect(result).toEqual(['b', 'c']);
expect(array).toEqual(['a', 'b', 'c']);
});
});

View File

@@ -1,3 +1,5 @@
import { toSpliced } from '~/utils/array/toSpliced';
/**
* Moves an item in an array from one index to another.
*
@@ -20,9 +22,14 @@ export const moveArrayItem = <ArrayItem>(
return array;
}
const arrayWithMovedItem = [...array];
const [itemToMove] = arrayWithMovedItem.splice(fromIndex, 1);
arrayWithMovedItem.splice(toIndex, 0, itemToMove);
const itemToMove = array[fromIndex];
const arrayWithoutItem = toSpliced(array, fromIndex, 1);
const arrayWithMovedItem = toSpliced(
arrayWithoutItem,
toIndex,
0,
itemToMove,
);
return arrayWithMovedItem;
};

View File

@@ -0,0 +1,28 @@
type ToSplicedFn = {
<T>(array: T[], start: number, deleteCount?: number): T[];
<T>(array: T[], start: number, deleteCount: number, ...items: T[]): T[];
};
/**
* Returns a new array with some elements removed and/or replaced at a given index.
* This does the same as `Array.prototype.toSpliced`.
* We cannot use the native `Array.prototype.toSpliced` method as of now because Chromatic's runners do not support it.
*
* @param array - The array to remove and/or replace elements from.
* @param start - The index at which to start changing the array.
* @param deleteCount - The number of elements in the array to remove from `start`.
* @param items - The elements to add to the array at `start`.
*
* @returns A new array with elements removed and/or replaced at a given index.
*
* @example
* toSpliced(['a', 'b', 'c'], 0, 1)
* => ['b', 'c']
* toSpliced(['a', 'b', 'c'], 0, 1, 'd')
* => ['d', 'b', 'c']
*/
export const toSpliced: ToSplicedFn = (array, ...args) => {
const arrayCopy = [...array];
arrayCopy.splice(...(args as [number, number, ...any[]]));
return arrayCopy;
};