Skip to content

Commit

Permalink
BC-7808 - Fix unexpected cloned card element (#3353)
Browse files Browse the repository at this point in the history
BC-7808 - implement key changing to force the component to re-rendering
  • Loading branch information
muratmerdoglu-dp authored Aug 19, 2024
1 parent 5b87a95 commit b4e65f6
Show file tree
Hide file tree
Showing 8 changed files with 98 additions and 7 deletions.
1 change: 0 additions & 1 deletion src/modules/data/board/Board.store.unit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -661,7 +661,6 @@ describe("BoardStore", () => {
expect(secondColumnCardsAfterMove?.map((card) => card.cardId)).toEqual([
secondCardId,
]);

expect(firstColumnCardsAfterMove?.map((card) => card.cardId)).toEqual([
firstCardId,
thirdCardId,
Expand Down
11 changes: 9 additions & 2 deletions src/modules/data/board/boardActions/boardSocketApi.composable.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as BoardActions from "./boardActions";
import * as CardActions from "../cardActions/cardActions";
import { useSocketConnection } from "@data-board";
import { useSocketConnection, useForceRender } from "@data-board";
import { useBoardStore } from "../Board.store";
import {
CreateCardRequestPayload,
Expand All @@ -9,6 +9,7 @@ import {
DisconnectSocketRequestPayload,
FetchBoardRequestPayload,
MoveCardRequestPayload,
MoveCardSuccessPayload,
MoveColumnRequestPayload,
UpdateBoardTitleRequestPayload,
UpdateBoardVisibilityRequestPayload,
Expand Down Expand Up @@ -95,7 +96,8 @@ export const useBoardSocketApi = () => {
...successActions,
...failureActions,
...ariaLiveNotifications,
on(BoardActions.disconnectSocket, disconnectSocketRequest)
on(BoardActions.disconnectSocket, disconnectSocketRequest),
on(BoardActions.moveCardSuccess, setRenderKeyAfterMoveCard)
);
};

Expand Down Expand Up @@ -171,6 +173,11 @@ export const useBoardSocketApi = () => {
emitOnSocket("update-board-visibility-request", payload);
};

const setRenderKeyAfterMoveCard = (payload: MoveCardSuccessPayload) => {
const { generateRenderKey } = useForceRender(payload.fromColumnId);
generateRenderKey();
};

const createCardFailure = () => notifySocketError("notCreated", "boardCard");
const createColumnFailure = () =>
notifySocketError("notCreated", "boardColumn");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,11 @@ import {
mockedPiniaStoreTyping,
} from "@@/tests/test-utils";
import setupStores from "@@/tests/test-utils/setupStores";
import { useBoardStore, useSocketConnection } from "@data-board";
import {
useBoardStore,
useSocketConnection,
useForceRender,
} from "@data-board";
import { createMock, DeepMocked } from "@golevelup/ts-jest";
import { createTestingPinia } from "@pinia/testing";
import { useBoardNotifier, useSharedLastCreatedElement } from "@util-board";
Expand All @@ -36,6 +40,9 @@ import { useBoardSocketApi } from "./boardSocketApi.composable";
jest.mock("../socket/socket");
const mockedUseSocketConnection = jest.mocked(useSocketConnection);

jest.mock("../fixSamePositionDnD.composable");
const mockedUseForceRender = jest.mocked(useForceRender);

jest.mock("./boardRestApi.composable");
const mockedUseBoardRestApi = jest.mocked(useBoardRestApi);

Expand All @@ -59,6 +66,7 @@ describe("useBoardSocketApi", () => {
let mockedSharedLastCreatedElementActions: DeepMocked<
ReturnType<typeof useSharedLastCreatedElement>
>;
let mockedUseForceRenderHandler: ReturnType<typeof useForceRender>;

beforeEach(() => {
setActivePinia(createTestingPinia());
Expand Down Expand Up @@ -89,6 +97,9 @@ describe("useBoardSocketApi", () => {
mockedSharedLastCreatedElement.mockReturnValue(
mockedSharedLastCreatedElementActions
);
mockedUseForceRenderHandler =
createMock<ReturnType<typeof useForceRender>>();
mockedUseForceRender.mockReturnValue(mockedUseForceRenderHandler);
});

it("should be defined", () => {
Expand Down Expand Up @@ -176,6 +187,7 @@ describe("useBoardSocketApi", () => {
dispatch(BoardActions.moveCardSuccess(payload));

expect(boardStore.moveCardSuccess).toHaveBeenCalledWith(payload);
expect(mockedUseForceRenderHandler.generateRenderKey).toHaveBeenCalled();
});

it("should call moveColumnSuccess for corresponding action", () => {
Expand Down
29 changes: 29 additions & 0 deletions src/modules/data/board/fixSamePositionDnD.composable.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import { reactive } from "vue";

const renderKeyList = reactive<Record<string, number>>({});

const getMaxRenderKey = () => {
return Object.values(renderKeyList).reduce((a, b) => Math.max(a, b), 0);
};

/*
* This artificial generating renderKey forces a component to re-render by changing the element key.
* It is used to fix issue described on https://ticketsystem.dbildungscloud.de/browse/BC-7806
*
* This solution is based on the article on https://michaelnthiessen.com/force-re-render/
*/
export const useForceRender = (id: string) => {
const generateRenderKey = () => {
renderKeyList[id] = getMaxRenderKey() + 1;
};

const getRenderKey = (): number => {
if (!renderKeyList[id]) generateRenderKey();
return renderKeyList[id];
};

return {
generateRenderKey,
getRenderKey,
};
};
16 changes: 16 additions & 0 deletions src/modules/data/board/fixSamePositionDnD.composable.unit.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { useForceRender } from "./fixSamePositionDnD.composable";

describe("fixSamePositionDnD", () => {
it("should set render key to the list", () => {
const { getRenderKey } = useForceRender("test-id-#1");
const renderKey = getRenderKey();
expect(renderKey).toBe(1);
});

it('should increase the render key when "generateRenderKey" is called', () => {
const { generateRenderKey, getRenderKey } = useForceRender("test-id-#1");
expect(getRenderKey()).toBe(1);
generateRenderKey();
expect(getRenderKey()).toBe(2);
});
});
2 changes: 2 additions & 0 deletions src/modules/data/board/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import * as cardActions from "./cardActions/cardActions";
import { useSocketConnection } from "./socket/socket";
import { useCardStore } from "./Card.store";
import { useBoardInactivity } from "./boardInactivity.composable";
import { useForceRender } from "./fixSamePositionDnD.composable";

export {
boardActions,
Expand All @@ -20,6 +21,7 @@ export {
useCardStore,
useContentElementState,
useEditMode,
useForceRender,
useSharedEditMode,
useSharedBoardPageInformation,
useSocketConnection,
Expand Down
19 changes: 18 additions & 1 deletion src/modules/feature/board/board/BoardColumn.unit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,11 @@ import {
envsFactory,
} from "@@/tests/test-utils/factory";
import { shallowMount } from "@vue/test-utils";
import { useBoardPermissions, useBoardStore } from "@data-board";
import {
useBoardPermissions,
useBoardStore,
useForceRender,
} from "@data-board";
import {
BoardPermissionChecks,
defaultPermissions,
Expand Down Expand Up @@ -32,8 +36,12 @@ const mockedUserPermissions = jest.mocked(useBoardPermissions);
jest.mock("@util-board");
const mockedUseBoardNotifier = jest.mocked(useBoardNotifier);

jest.mock("@data-board/fixSamePositionDnD.composable");
const mockedUseForceRender = jest.mocked(useForceRender);

describe("BoardColumn", () => {
let mockedBoardNotifierCalls: DeepMocked<ReturnType<typeof useBoardNotifier>>;
let mockedUseForceRenderHandler: ReturnType<typeof useForceRender>;

beforeEach(() => {
setupStores({ envConfigModule: EnvConfigModule });
Expand All @@ -45,6 +53,10 @@ describe("BoardColumn", () => {
mockedBoardNotifierCalls =
createMock<ReturnType<typeof useBoardNotifier>>();
mockedUseBoardNotifier.mockReturnValue(mockedBoardNotifierCalls);

mockedUseForceRenderHandler =
createMock<ReturnType<typeof useForceRender>>();
mockedUseForceRender.mockReturnValue(mockedUseForceRenderHandler);
});

const setup = (options?: {
Expand Down Expand Up @@ -93,6 +105,11 @@ describe("BoardColumn", () => {
const { wrapper } = setup();
expect(wrapper.findComponent(BoardColumnVue).exists()).toBe(true);
});

it("should trigger 'getRenderKey' method", () => {
setup();
expect(mockedUseForceRenderHandler.getRenderKey).toHaveBeenCalled();
});
});

describe("when a card moved ", () => {
Expand Down
13 changes: 11 additions & 2 deletions src/modules/feature/board/board/BoardColumn.vue
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<template>
<div :style="columnStyle" :class="columnClasses">
<div :style="columnStyle" :class="columnClasses" :key="renderKey">
<BoardColumnHeader
:columnId="column.id"
:title="column.title"
Expand Down Expand Up @@ -72,7 +72,11 @@ import { useDebounceFn } from "@vueuse/core";
import { PropType, computed, defineComponent, provide, ref, toRef } from "vue";
import CardHost from "../card/CardHost.vue";
import { useDragAndDrop } from "../shared/DragAndDrop.composable";
import { useBoardPermissions, useBoardStore } from "@data-board";
import {
useBoardPermissions,
useBoardStore,
useForceRender,
} from "@data-board";
import { useTouchDetection } from "@util-device-detection";
import { BoardColumn, BoardSkeletonCard } from "@/types/board/Board";
import {
Expand Down Expand Up @@ -311,6 +315,10 @@ export default defineComponent({
return classes;
});
const columnId = toRef(props, "column").value.id;
const { getRenderKey } = useForceRender(columnId);
const renderKey = computed(() => getRenderKey());
return {
cardDropPlaceholderOptions,
columnClasses,
Expand All @@ -335,6 +343,7 @@ export default defineComponent({
onUpdateTitle,
getChildPayload,
reactiveIndex,
renderKey,
showAddButton,
sortableGhostClasses,
};
Expand Down

0 comments on commit b4e65f6

Please sign in to comment.