Skip to content

Commit

Permalink
fix: avoid freeze of the ui when board size value is invalid
Browse files Browse the repository at this point in the history
Refs: SHELL-153 (#413)
  • Loading branch information
beawar authored May 3, 2024
1 parent c086403 commit cdefb10
Show file tree
Hide file tree
Showing 8 changed files with 37 additions and 36 deletions.
2 changes: 1 addition & 1 deletion api-extractor/carbonio-shell-ui.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -915,7 +915,7 @@ export const LOCAL_STORAGE_SETTINGS_KEY = "settings";

// @public (undocumented)
type LocalStorageOptions = {
keepSynchedWithStorage?: boolean;
keepSyncedWithStorage?: boolean;
};

// @public (undocumented)
Expand Down
4 changes: 2 additions & 2 deletions src/shell/boards/board-container.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -489,7 +489,7 @@ describe('Board container', () => {
jest.advanceTimersToNextTimer();
});
await waitFor(() =>
expect(JSON.parse(window.localStorage.getItem(LOCAL_STORAGE_BOARD_SIZE) || '')).toEqual({})
expect(JSON.parse(window.localStorage.getItem(LOCAL_STORAGE_BOARD_SIZE) ?? '')).toEqual({})
);
expect(board).toHaveStyle({
height: '70vh',
Expand Down Expand Up @@ -1017,7 +1017,7 @@ describe('Board container', () => {
setupBoardStore();
});
await waitFor(() =>
expect(JSON.parse(window.localStorage.getItem(LOCAL_STORAGE_BOARD_SIZE) || '{}')).toEqual({})
expect(JSON.parse(window.localStorage.getItem(LOCAL_STORAGE_BOARD_SIZE) ?? '{}')).toEqual({})
);
expect(getByRoleWithIcon('button', { icon: ICONS.resetBoardSize })).toBeDisabled();
});
Expand Down
16 changes: 8 additions & 8 deletions src/shell/boards/resizable-container.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@ interface ResizableBorderProps {
border: Border;
elementToResize: React.RefObject<HTMLElement>;
localStorageKey?: string;
keepSynchedWithStorage?: boolean;
keepSyncedWithStorage?: boolean;
}

interface ResizableContainerProps extends ContainerProps {
elementToResize: React.RefObject<HTMLElement>;
localStorageKey?: string;
keepSynchedWithStorage?: boolean;
keepSyncedWithStorage?: boolean;
disabled?: boolean;
minSize?: { width: number; height: number };
}
Expand Down Expand Up @@ -63,20 +63,20 @@ const BorderWithResize = styled.div<
${({ $translateTransform }): SimpleInterpolation =>
($translateTransform?.x || $translateTransform?.y) &&
css`
transform: translate(${$translateTransform?.x || 0}, ${$translateTransform?.y || 0});
transform: translate(${$translateTransform?.x ?? 0}, ${$translateTransform?.y ?? 0});
`}
`;

const ResizableBorder = ({
border,
elementToResize,
localStorageKey,
keepSynchedWithStorage
keepSyncedWithStorage
}: ResizableBorderProps): React.JSX.Element => {
const borderRef = useRef<HTMLDivElement>(null);
const resizeHandler = useResize(elementToResize, border, {
localStorageKey,
keepSynchedWithStorage
keepSyncedWithStorage
});

const sizes = useMemo<Pick<BorderWithResizeProps, '$width' | '$height'>>(() => {
Expand Down Expand Up @@ -146,7 +146,7 @@ export const ResizableContainer = ({
children,
localStorageKey,
disabled = false,
keepSynchedWithStorage,
keepSyncedWithStorage,
...rest
}: ResizableContainerProps): React.JSX.Element => {
const borders = useMemo(
Expand All @@ -157,10 +157,10 @@ export const ResizableContainer = ({
border={border}
elementToResize={elementToResize}
localStorageKey={localStorageKey}
keepSynchedWithStorage={keepSynchedWithStorage}
keepSyncedWithStorage={keepSyncedWithStorage}
/>
)),
[elementToResize, keepSynchedWithStorage, localStorageKey]
[elementToResize, keepSyncedWithStorage, localStorageKey]
);

return (
Expand Down
2 changes: 1 addition & 1 deletion src/shell/hooks/useLocalStorage.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ describe('use local storage', () => {
const initial = 'initial';
const updatesLocalStorage = [initial];
const updatesLocalStorageStore = [undefined, initial];
controlConsoleError('Unexpected token o in JSON at position 1');
controlConsoleError('Cannot read local storage test-key with value "not a JSON":');
setup(
<TestComponent initialValue={initial} updatedValue={'updated'} localStorageKey={lsKey} />
);
Expand Down
31 changes: 16 additions & 15 deletions src/shell/hooks/useLocalStorage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ function isSameLocalStorageValue(valueA: unknown, valueB: unknown): boolean {
return JSON.stringify(valueA) === JSON.stringify(valueB);
}

type LocalStorageOptions = { keepSynchedWithStorage?: boolean };
type LocalStorageOptions = { keepSyncedWithStorage?: boolean };

type LocalStorageState = {
storage: Record<string, unknown>;
Expand All @@ -25,19 +25,20 @@ type LocalStorageState = {
const useLocalStorageStore = create<LocalStorageState>()((setState) => ({
storage: {},
readValue<T>(key: string, fallback: T): void {
const localStorageItem = window.localStorage.getItem(key);
let item: T;
try {
const localStorageItem = window.localStorage.getItem(key);
const item = localStorageItem !== null ? JSON.parse(localStorageItem) : fallback;
setState((state) => {
if (state.storage[key] === undefined) {
return { storage: { ...state.storage, [key]: item } };
}
return state;
});
item = localStorageItem !== null ? JSON.parse(localStorageItem) : fallback;
} catch (error) {
console.error(error);
setState((state) => ({ storage: { ...state.storage, [key]: fallback } }));
console.error(`Cannot read local storage ${key} with value "${localStorageItem}":`, error);
item = fallback;
}
setState((state) => {
if (state.storage[key] === undefined) {
return { storage: { ...state.storage, [key]: item } };
}
return state;
});
},
setValue<T>(key: string, value: React.SetStateAction<T>): void {
setState((state) => {
Expand All @@ -52,15 +53,15 @@ const useLocalStorageStore = create<LocalStorageState>()((setState) => ({
}));

const DEFAULT_OPTIONS: LocalStorageOptions = {
keepSynchedWithStorage: true
keepSyncedWithStorage: true
};

export function useLocalStorage<T>(
key: string,
initialValue: T,
options = DEFAULT_OPTIONS
): [T, React.Dispatch<React.SetStateAction<T>>] {
const storedValue = useLocalStorageStore((state) => (state.storage[key] as T) || initialValue);
const storedValue = useLocalStorageStore((state) => (state.storage[key] as T) ?? initialValue);
const shouldDispatchStorageEventRef = useRef(false);
const localStorageOptions = useMemo(() => ({ ...DEFAULT_OPTIONS, ...options }), [options]);

Expand All @@ -81,14 +82,14 @@ export function useLocalStorage<T>(
);

useEffect(() => {
if (localStorageOptions?.keepSynchedWithStorage) {
if (localStorageOptions?.keepSyncedWithStorage) {
window.addEventListener('storage', readValueForKey);
}

return (): void => {
window.removeEventListener('storage', readValueForKey);
};
}, [localStorageOptions?.keepSynchedWithStorage, readValueForKey]);
}, [localStorageOptions?.keepSyncedWithStorage, readValueForKey]);

useEffect(() => {
if (shouldDispatchStorageEventRef.current) {
Expand Down
4 changes: 2 additions & 2 deletions src/shell/hooks/useMove.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { setElementSizeAndPosition, setGlobalCursor } from '../../utils/utils';
type UseMoveReturnType = [isMoving: boolean, moveHandler: React.MouseEventHandler];
type MoveOptions = {
localStorageKey?: string;
keepSynchedWithStorage?: boolean;
keepSyncedWithStorage?: boolean;
};

export const BOARD_CURSOR_TIMEOUT = 250;
Expand Down Expand Up @@ -55,7 +55,7 @@ export const useMove = (
const [lastSavedPosition, setLastSavedPosition] = useLocalStorage<Partial<SizeAndPosition>>(
options?.localStorageKey || 'use-move-data',
{},
{ keepSynchedWithStorage: options?.keepSynchedWithStorage }
{ keepSyncedWithStorage: options?.keepSyncedWithStorage }
);
const lastPositionRef = useRef<Partial<ElementPosition>>(lastSavedPosition);
const globalCursorSetterTimerRef = useRef<ReturnType<typeof setTimeout>>();
Expand Down
8 changes: 4 additions & 4 deletions src/shell/hooks/useResize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ type UseResizableReturnType = React.MouseEventHandler;

type ResizeOptions = {
localStorageKey?: string;
keepSynchedWithStorage?: boolean;
keepSyncedWithStorage?: boolean;
};

export const BORDERS: Border[] = ['n', 's', 'e', 'w', 'ne', 'se', 'sw', 'nw'];
Expand All @@ -42,7 +42,7 @@ export function getCursorFromBorder(border: Border): NonNullable<CSSProperties['
],
(borders) => borders.includes(border)
)?.join('');
return (direction && direction.concat('-resize')) || '';
return direction?.concat('-resize') ?? '';
}

function calcNewSizeAndPosition(
Expand Down Expand Up @@ -84,9 +84,9 @@ export const useResize = (
const [lastSavedSizeAndPosition, setLastSavedSizeAndPosition] = useLocalStorage<
Partial<SizeAndPosition>
>(
options?.localStorageKey || 'use-resize-data',
options?.localStorageKey ?? 'use-resize-data',
{},
{ keepSynchedWithStorage: options?.keepSynchedWithStorage }
{ keepSyncedWithStorage: options?.keepSyncedWithStorage }
);
const lastSizeAndPositionRef = useRef<Partial<SizeAndPosition>>(lastSavedSizeAndPosition);

Expand Down
6 changes: 3 additions & 3 deletions src/test/test-board-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export function setupBoardStore(current?: string, boardState?: Record<string, Bo
useBoardStore.setState(() => ({
boards,
orderedBoards: Object.keys(boards),
current: current || first(keys(boards))
current: current ?? first(keys(boards))
}));
}

Expand Down Expand Up @@ -123,7 +123,7 @@ export async function resizeBoard(
// eslint-disable-next-line testing-library/prefer-user-event
fireEvent.mouseUp(document.body);
await waitFor(() =>
expect(JSON.parse(window.localStorage.getItem(LOCAL_STORAGE_BOARD_SIZE) || '{}')).toEqual(
expect(JSON.parse(window.localStorage.getItem(LOCAL_STORAGE_BOARD_SIZE) ?? '{}')).toEqual(
boardNewPosition
)
);
Expand Down Expand Up @@ -155,7 +155,7 @@ export async function moveBoard(
// eslint-disable-next-line testing-library/prefer-user-event
fireEvent.click(elementForMove);
await waitFor(() =>
expect(JSON.parse(window.localStorage.getItem(LOCAL_STORAGE_BOARD_SIZE) || '{}')).toEqual(
expect(JSON.parse(window.localStorage.getItem(LOCAL_STORAGE_BOARD_SIZE) ?? '{}')).toEqual(
boardNewPosition
)
);
Expand Down

0 comments on commit cdefb10

Please sign in to comment.