From b88d08737ec8938890491d5cda10e7df7258b884 Mon Sep 17 00:00:00 2001 From: Cataldo Mazzilli Date: Mon, 3 Apr 2023 12:29:58 +0200 Subject: [PATCH 1/3] fix: use fallback closing board tab refs: SHELL-73 --- src/shell/boards/board-tab-list.tsx | 20 +++++++++++++++++--- src/shell/boards/board-tab.tsx | 10 +++++++--- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/src/shell/boards/board-tab-list.tsx b/src/shell/boards/board-tab-list.tsx index 7cfeed6b..5a541c98 100644 --- a/src/shell/boards/board-tab-list.tsx +++ b/src/shell/boards/board-tab-list.tsx @@ -39,9 +39,23 @@ export const TabsList: FC = () => { width="calc(100% - 0.5rem)" > {boards && - map(boards, (tab) => ( - - ))} + map( + map(boards, (board) => board), + (tab, index, array) => ( + + ) + )} {hiddenTabsCount > 0 && ( <> diff --git a/src/shell/boards/board-tab.tsx b/src/shell/boards/board-tab.tsx index c6ea5b0b..adecacc6 100644 --- a/src/shell/boards/board-tab.tsx +++ b/src/shell/boards/board-tab.tsx @@ -37,10 +37,11 @@ const VerticalDivider = styled(Container)` margin: ${({ theme }): string => theme.sizes.padding.extrasmall}; `; -export const AppBoardTab: FC<{ id: string; icon: string; title: string }> = ({ +export const AppBoardTab: FC<{ id: string; icon: string; title: string; fallbackId?: string }> = ({ id, icon, - title + title, + fallbackId }) => { const current = useBoardStore((s) => s.current); const t = getT(); @@ -49,8 +50,11 @@ export const AppBoardTab: FC<{ id: string; icon: string; title: string }> = ({ (ev) => { ev.stopPropagation(); closeBoard(id); + if (fallbackId) { + setCurrentBoard(fallbackId); + } }, - [id] + [fallbackId, id] ); return ( From dbdd690a78bc15be443e95dcd6e29fb268756ff5 Mon Sep 17 00:00:00 2001 From: Cataldo Mazzilli Date: Tue, 4 Apr 2023 16:59:34 +0200 Subject: [PATCH 2/3] test: add tests on fallback tab refs: SHELL-73 --- src/mocks/handlers.ts | 3 +- src/shell/boards/board-tab-list.test.tsx | 147 +++++++++++++++++++++++ src/shell/boards/board-tab.tsx | 6 +- 3 files changed, 154 insertions(+), 2 deletions(-) create mode 100644 src/shell/boards/board-tab-list.test.tsx diff --git a/src/mocks/handlers.ts b/src/mocks/handlers.ts index 694d330d..7512365d 100644 --- a/src/mocks/handlers.ts +++ b/src/mocks/handlers.ts @@ -10,7 +10,8 @@ import { getInfoRequest } from './handlers/getInfoRequest'; const handlers: RequestHandler[] = [ rest.get('/static/iris/components.json', getComponentsJson), - rest.post('/service/soap/GetInfoRequest', getInfoRequest) + rest.post('/service/soap/GetInfoRequest', getInfoRequest), + rest.get('/i18n/en.json', (request, response, context) => response(context.json({}))) ]; export default handlers; diff --git a/src/shell/boards/board-tab-list.test.tsx b/src/shell/boards/board-tab-list.test.tsx new file mode 100644 index 00000000..bd9f2018 --- /dev/null +++ b/src/shell/boards/board-tab-list.test.tsx @@ -0,0 +1,147 @@ +/* + * SPDX-FileCopyrightText: 2022 Zextras + * + * SPDX-License-Identifier: AGPL-3.0-only + */ + +import React from 'react'; +import { screen, within } from '@testing-library/react'; +import { setup } from '../../test/utils'; +import { TabsList } from './board-tab-list'; +import { useBoardStore } from '../../store/boards'; +import { Board } from '../../../types'; + +describe('Shell boards', () => { + const boards: Record = { + 'board-1': { id: 'board-1', url: 'url', app: 'app', title: 'title1', icon: 'CubeOutline' }, + 'board-2': { id: 'board-2', url: 'url', app: 'app', title: 'title2', icon: 'CubeOutline' }, + 'board-3': { id: 'board-3', url: 'url', app: 'app', title: 'title3', icon: 'CubeOutline' } + }; + + function setupState(current: string, boardState?: Record): void { + useBoardStore.setState(() => ({ + boards: boardState || boards, + current + })); + } + + test('If I close the first tab that is open, the tab on its right should be seen correctly', async () => { + setupState('board-1'); + const { user } = setup(); + const title1 = screen.getByText('title1'); + const title2 = screen.getByText('title2'); + const title3 = screen.getByText('title3'); + expect(title1).toHaveAttribute('color', 'text'); + expect(title2).toHaveAttribute('color', 'secondary'); + expect(title3).toHaveAttribute('color', 'secondary'); + + const tab1 = screen.getByTestId(`board-tab-board-1`); + const board1closeIcon = within(tab1).getByTestId('icon: Close'); + await user.click(board1closeIcon); + expect(tab1).not.toBeInTheDocument(); + expect(title1).not.toBeInTheDocument(); + expect(title2).toHaveAttribute('color', 'text'); + expect(title3).toHaveAttribute('color', 'secondary'); + expect(useBoardStore.getState().current).toBe('board-2'); + }); + + test('If I close the first tab that is not open, the one that is already open must remain correctly visible', async () => { + setupState('board-2'); + const { user } = setup(); + const title1 = screen.getByText('title1'); + const title2 = screen.getByText('title2'); + const title3 = screen.getByText('title3'); + expect(title1).toHaveAttribute('color', 'secondary'); + expect(title2).toHaveAttribute('color', 'text'); + expect(title3).toHaveAttribute('color', 'secondary'); + + const tab1 = screen.getByTestId(`board-tab-board-1`); + const board1closeIcon = within(tab1).getByTestId('icon: Close'); + await user.click(board1closeIcon); + expect(tab1).not.toBeInTheDocument(); + expect(title1).not.toBeInTheDocument(); + expect(title2).toHaveAttribute('color', 'text'); + expect(title3).toHaveAttribute('color', 'secondary'); + expect(useBoardStore.getState().current).toBe('board-2'); + }); + + test('If I close the last tab that is open (tab on the far right) the tab on its left must be correctly visible', async () => { + setupState('board-3'); + const { user } = setup(); + const title1 = screen.getByText('title1'); + const title2 = screen.getByText('title2'); + const title3 = screen.getByText('title3'); + expect(title1).toHaveAttribute('color', 'secondary'); + expect(title2).toHaveAttribute('color', 'secondary'); + expect(title3).toHaveAttribute('color', 'text'); + + const tab3 = screen.getByTestId(`board-tab-board-3`); + const board3closeIcon = within(tab3).getByTestId('icon: Close'); + await user.click(board3closeIcon); + expect(tab3).not.toBeInTheDocument(); + expect(title3).not.toBeInTheDocument(); + expect(title1).toHaveAttribute('color', 'secondary'); + expect(title2).toHaveAttribute('color', 'text'); + expect(useBoardStore.getState().current).toBe('board-2'); + }); + + test('If I close the last tab that is not open (tab on the far right) the one that is already open must remain correctly visible', async () => { + setupState('board-2'); + const { user } = setup(); + const title1 = screen.getByText('title1'); + const title2 = screen.getByText('title2'); + const title3 = screen.getByText('title3'); + expect(title1).toHaveAttribute('color', 'secondary'); + expect(title2).toHaveAttribute('color', 'text'); + expect(title3).toHaveAttribute('color', 'secondary'); + + const tab3 = screen.getByTestId(`board-tab-board-3`); + const board3closeIcon = within(tab3).getByTestId('icon: Close'); + await user.click(board3closeIcon); + expect(tab3).not.toBeInTheDocument(); + expect(title3).not.toBeInTheDocument(); + expect(title1).toHaveAttribute('color', 'secondary'); + expect(title2).toHaveAttribute('color', 'text'); + expect(useBoardStore.getState().current).toBe('board-2'); + }); + + test('If i close a middle tab that is open, the tab on its right must be correctly visible', async () => { + setupState('board-2'); + const { user } = setup(); + const title1 = screen.getByText('title1'); + const title2 = screen.getByText('title2'); + const title3 = screen.getByText('title3'); + expect(title1).toHaveAttribute('color', 'secondary'); + expect(title2).toHaveAttribute('color', 'text'); + expect(title3).toHaveAttribute('color', 'secondary'); + + const tab2 = screen.getByTestId(`board-tab-board-2`); + const board2closeIcon = within(tab2).getByTestId('icon: Close'); + await user.click(board2closeIcon); + expect(tab2).not.toBeInTheDocument(); + expect(title2).not.toBeInTheDocument(); + expect(title1).toHaveAttribute('color', 'secondary'); + expect(title3).toHaveAttribute('color', 'text'); + expect(useBoardStore.getState().current).toBe('board-3'); + }); + + test('If I close a middle tab that is not open, the one that is already open must remain correctly visible', async () => { + setupState('board-1'); + const { user } = setup(); + const title1 = screen.getByText('title1'); + const title2 = screen.getByText('title2'); + const title3 = screen.getByText('title3'); + expect(title1).toHaveAttribute('color', 'text'); + expect(title2).toHaveAttribute('color', 'secondary'); + expect(title3).toHaveAttribute('color', 'secondary'); + + const tab2 = screen.getByTestId(`board-tab-board-2`); + const board2closeIcon = within(tab2).getByTestId('icon: Close'); + await user.click(board2closeIcon); + expect(tab2).not.toBeInTheDocument(); + expect(title2).not.toBeInTheDocument(); + expect(title1).toHaveAttribute('color', 'text'); + expect(title3).toHaveAttribute('color', 'secondary'); + expect(useBoardStore.getState().current).toBe('board-1'); + }); +}); diff --git a/src/shell/boards/board-tab.tsx b/src/shell/boards/board-tab.tsx index adecacc6..8756efa3 100644 --- a/src/shell/boards/board-tab.tsx +++ b/src/shell/boards/board-tab.tsx @@ -60,7 +60,11 @@ export const AppBoardTab: FC<{ id: string; icon: string; title: string; fallback return ( {current !== id && } - + Date: Thu, 6 Apr 2023 10:01:10 +0200 Subject: [PATCH 3/3] fix: apply review hints refs: SHELL-73 --- package-lock.json | 25 ++++++ package.json | 1 + src/shell/boards/board-tab-list.test.tsx | 63 +++++++------- src/shell/boards/board-tab-list.tsx | 43 ++++------ src/store/boards/store.ts | 65 ++++++++------- src/test/constants.ts | 101 +++++++++++++++++++++++ types/boards/index.d.ts | 1 + 7 files changed, 213 insertions(+), 86 deletions(-) diff --git a/package-lock.json b/package-lock.json index fcb4d218..4a06025f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -73,6 +73,7 @@ "jest-environment-jsdom": "^29.5.0", "jest-fail-on-console": "^3.1.1", "jest-junit": "^15.0.0", + "jest-styled-components": "^7.1.1", "mini-css-extract-plugin": "^2.7.5", "moment": "^2.29.4", "msw": "^1.2.1", @@ -13024,6 +13025,21 @@ "node": ">=8" } }, + "node_modules/jest-styled-components": { + "version": "7.1.1", + "resolved": "https://registry.npmjs.org/jest-styled-components/-/jest-styled-components-7.1.1.tgz", + "integrity": "sha512-OUq31R5CivBF8oy81dnegNQrRW13TugMol/Dz6ZnFfEyo03exLASod7YGwyHGuayYlKmCstPtz0RQ1+NrAbIIA==", + "dev": true, + "dependencies": { + "@adobe/css-tools": "^4.0.1" + }, + "engines": { + "node": ">= 12" + }, + "peerDependencies": { + "styled-components": ">= 5" + } + }, "node_modules/jest-util": { "version": "29.5.0", "resolved": "https://registry.npmjs.org/jest-util/-/jest-util-29.5.0.tgz", @@ -28192,6 +28208,15 @@ } } }, + "jest-styled-components": { + "version": "7.1.1", + "resolved": "https://registry.npmjs.org/jest-styled-components/-/jest-styled-components-7.1.1.tgz", + "integrity": "sha512-OUq31R5CivBF8oy81dnegNQrRW13TugMol/Dz6ZnFfEyo03exLASod7YGwyHGuayYlKmCstPtz0RQ1+NrAbIIA==", + "dev": true, + "requires": { + "@adobe/css-tools": "^4.0.1" + } + }, "jest-util": { "version": "29.5.0", "resolved": "https://registry.npmjs.org/jest-util/-/jest-util-29.5.0.tgz", diff --git a/package.json b/package.json index 9ee5b53d..75c48068 100644 --- a/package.json +++ b/package.json @@ -77,6 +77,7 @@ "jest-environment-jsdom": "^29.5.0", "jest-fail-on-console": "^3.1.1", "jest-junit": "^15.0.0", + "jest-styled-components": "^7.1.1", "mini-css-extract-plugin": "^2.7.5", "moment": "^2.29.4", "msw": "^1.2.1", diff --git a/src/shell/boards/board-tab-list.test.tsx b/src/shell/boards/board-tab-list.test.tsx index bd9f2018..5ab943b5 100644 --- a/src/shell/boards/board-tab-list.test.tsx +++ b/src/shell/boards/board-tab-list.test.tsx @@ -10,6 +10,8 @@ import { setup } from '../../test/utils'; import { TabsList } from './board-tab-list'; import { useBoardStore } from '../../store/boards'; import { Board } from '../../../types'; +import 'jest-styled-components'; +import { PALETTE } from '../../test/constants'; describe('Shell boards', () => { const boards: Record = { @@ -21,6 +23,7 @@ describe('Shell boards', () => { function setupState(current: string, boardState?: Record): void { useBoardStore.setState(() => ({ boards: boardState || boards, + orderedBoards: ['board-1', 'board-2', 'board-3'], current })); } @@ -31,17 +34,17 @@ describe('Shell boards', () => { const title1 = screen.getByText('title1'); const title2 = screen.getByText('title2'); const title3 = screen.getByText('title3'); - expect(title1).toHaveAttribute('color', 'text'); - expect(title2).toHaveAttribute('color', 'secondary'); - expect(title3).toHaveAttribute('color', 'secondary'); + expect(title1).toHaveStyleRule('color', PALETTE.text.regular); + expect(title2).toHaveStyleRule('color', PALETTE.secondary.regular); + expect(title3).toHaveStyleRule('color', PALETTE.secondary.regular); const tab1 = screen.getByTestId(`board-tab-board-1`); const board1closeIcon = within(tab1).getByTestId('icon: Close'); await user.click(board1closeIcon); expect(tab1).not.toBeInTheDocument(); expect(title1).not.toBeInTheDocument(); - expect(title2).toHaveAttribute('color', 'text'); - expect(title3).toHaveAttribute('color', 'secondary'); + expect(title2).toHaveStyleRule('color', PALETTE.text.regular); + expect(title3).toHaveStyleRule('color', PALETTE.secondary.regular); expect(useBoardStore.getState().current).toBe('board-2'); }); @@ -51,17 +54,17 @@ describe('Shell boards', () => { const title1 = screen.getByText('title1'); const title2 = screen.getByText('title2'); const title3 = screen.getByText('title3'); - expect(title1).toHaveAttribute('color', 'secondary'); - expect(title2).toHaveAttribute('color', 'text'); - expect(title3).toHaveAttribute('color', 'secondary'); + expect(title1).toHaveStyleRule('color', PALETTE.secondary.regular); + expect(title2).toHaveStyleRule('color', PALETTE.text.regular); + expect(title3).toHaveStyleRule('color', PALETTE.secondary.regular); const tab1 = screen.getByTestId(`board-tab-board-1`); const board1closeIcon = within(tab1).getByTestId('icon: Close'); await user.click(board1closeIcon); expect(tab1).not.toBeInTheDocument(); expect(title1).not.toBeInTheDocument(); - expect(title2).toHaveAttribute('color', 'text'); - expect(title3).toHaveAttribute('color', 'secondary'); + expect(title2).toHaveStyleRule('color', PALETTE.text.regular); + expect(title3).toHaveStyleRule('color', PALETTE.secondary.regular); expect(useBoardStore.getState().current).toBe('board-2'); }); @@ -71,17 +74,17 @@ describe('Shell boards', () => { const title1 = screen.getByText('title1'); const title2 = screen.getByText('title2'); const title3 = screen.getByText('title3'); - expect(title1).toHaveAttribute('color', 'secondary'); - expect(title2).toHaveAttribute('color', 'secondary'); - expect(title3).toHaveAttribute('color', 'text'); + expect(title1).toHaveStyleRule('color', PALETTE.secondary.regular); + expect(title2).toHaveStyleRule('color', PALETTE.secondary.regular); + expect(title3).toHaveStyleRule('color', PALETTE.text.regular); const tab3 = screen.getByTestId(`board-tab-board-3`); const board3closeIcon = within(tab3).getByTestId('icon: Close'); await user.click(board3closeIcon); expect(tab3).not.toBeInTheDocument(); expect(title3).not.toBeInTheDocument(); - expect(title1).toHaveAttribute('color', 'secondary'); - expect(title2).toHaveAttribute('color', 'text'); + expect(title1).toHaveStyleRule('color', PALETTE.secondary.regular); + expect(title2).toHaveStyleRule('color', PALETTE.text.regular); expect(useBoardStore.getState().current).toBe('board-2'); }); @@ -91,17 +94,17 @@ describe('Shell boards', () => { const title1 = screen.getByText('title1'); const title2 = screen.getByText('title2'); const title3 = screen.getByText('title3'); - expect(title1).toHaveAttribute('color', 'secondary'); - expect(title2).toHaveAttribute('color', 'text'); - expect(title3).toHaveAttribute('color', 'secondary'); + expect(title1).toHaveStyleRule('color', PALETTE.secondary.regular); + expect(title2).toHaveStyleRule('color', PALETTE.text.regular); + expect(title3).toHaveStyleRule('color', PALETTE.secondary.regular); const tab3 = screen.getByTestId(`board-tab-board-3`); const board3closeIcon = within(tab3).getByTestId('icon: Close'); await user.click(board3closeIcon); expect(tab3).not.toBeInTheDocument(); expect(title3).not.toBeInTheDocument(); - expect(title1).toHaveAttribute('color', 'secondary'); - expect(title2).toHaveAttribute('color', 'text'); + expect(title1).toHaveStyleRule('color', PALETTE.secondary.regular); + expect(title2).toHaveStyleRule('color', PALETTE.text.regular); expect(useBoardStore.getState().current).toBe('board-2'); }); @@ -111,17 +114,17 @@ describe('Shell boards', () => { const title1 = screen.getByText('title1'); const title2 = screen.getByText('title2'); const title3 = screen.getByText('title3'); - expect(title1).toHaveAttribute('color', 'secondary'); - expect(title2).toHaveAttribute('color', 'text'); - expect(title3).toHaveAttribute('color', 'secondary'); + expect(title1).toHaveStyleRule('color', PALETTE.secondary.regular); + expect(title2).toHaveStyleRule('color', PALETTE.text.regular); + expect(title3).toHaveStyleRule('color', PALETTE.secondary.regular); const tab2 = screen.getByTestId(`board-tab-board-2`); const board2closeIcon = within(tab2).getByTestId('icon: Close'); await user.click(board2closeIcon); expect(tab2).not.toBeInTheDocument(); expect(title2).not.toBeInTheDocument(); - expect(title1).toHaveAttribute('color', 'secondary'); - expect(title3).toHaveAttribute('color', 'text'); + expect(title1).toHaveStyleRule('color', PALETTE.secondary.regular); + expect(title3).toHaveStyleRule('color', PALETTE.text.regular); expect(useBoardStore.getState().current).toBe('board-3'); }); @@ -131,17 +134,17 @@ describe('Shell boards', () => { const title1 = screen.getByText('title1'); const title2 = screen.getByText('title2'); const title3 = screen.getByText('title3'); - expect(title1).toHaveAttribute('color', 'text'); - expect(title2).toHaveAttribute('color', 'secondary'); - expect(title3).toHaveAttribute('color', 'secondary'); + expect(title1).toHaveStyleRule('color', PALETTE.text.regular); + expect(title2).toHaveStyleRule('color', PALETTE.secondary.regular); + expect(title3).toHaveStyleRule('color', PALETTE.secondary.regular); const tab2 = screen.getByTestId(`board-tab-board-2`); const board2closeIcon = within(tab2).getByTestId('icon: Close'); await user.click(board2closeIcon); expect(tab2).not.toBeInTheDocument(); expect(title2).not.toBeInTheDocument(); - expect(title1).toHaveAttribute('color', 'text'); - expect(title3).toHaveAttribute('color', 'secondary'); + expect(title1).toHaveStyleRule('color', PALETTE.text.regular); + expect(title3).toHaveStyleRule('color', PALETTE.secondary.regular); expect(useBoardStore.getState().current).toBe('board-1'); }); }); diff --git a/src/shell/boards/board-tab-list.tsx b/src/shell/boards/board-tab-list.tsx index 5a541c98..a3555e8d 100644 --- a/src/shell/boards/board-tab-list.tsx +++ b/src/shell/boards/board-tab-list.tsx @@ -12,7 +12,7 @@ import { Tooltip, useHiddenCount } from '@zextras/carbonio-design-system'; -import { map, noop, slice } from 'lodash'; +import { isEmpty, map, noop, slice } from 'lodash'; import React, { FC, useLayoutEffect, useRef } from 'react'; import { setCurrentBoard, useBoardStore } from '../../store/boards'; import { getT } from '../../store/i18n'; @@ -20,7 +20,7 @@ import { AppBoardTab } from './board-tab'; export const TabsList: FC = () => { const t = getT(); - const { boards, current, expanded } = useBoardStore(); + const { boards, current, expanded, orderedBoards } = useBoardStore(); const tabContainerRef = useRef(null); const [hiddenTabsCount, recalculateHiddenTabs] = useHiddenCount(tabContainerRef, expanded); @@ -39,23 +39,16 @@ export const TabsList: FC = () => { width="calc(100% - 0.5rem)" > {boards && - map( - map(boards, (board) => board), - (tab, index, array) => ( - - ) - )} + !isEmpty(orderedBoards) && + map(orderedBoards, (boardId, index, array) => ( + + ))} {hiddenTabsCount > 0 && ( <> @@ -66,12 +59,12 @@ export const TabsList: FC = () => { ({ - id: tab.id, - label: tab.title, - icon: tab.icon, - click: () => setCurrentBoard(tab.id), - selected: tab.id === current + items={map(slice(orderedBoards, -hiddenTabsCount), (boardId) => ({ + id: boardId, + label: boards[boardId].title, + icon: boards[boardId].icon, + click: () => setCurrentBoard(boardId), + selected: boardId === current }))} >