From 62b0c4648345e86d10dd4c080006d337bdc445a4 Mon Sep 17 00:00:00 2001 From: Alexis Demetriou Date: Wed, 21 Aug 2024 17:05:53 +0300 Subject: [PATCH 1/5] Fixed active sidebar menu is highlighted for the correct Kafka instance --- frontend/src/components/ACLPage/List/List.tsx | 2 +- .../src/components/Brokers/Broker/Broker.tsx | 1 + .../Brokers/BrokersList/BrokersList.tsx | 2 +- .../Connect/Details/DetailsPage.tsx | 1 + .../src/components/Connect/List/ListPage.tsx | 2 +- frontend/src/components/Connect/New/New.tsx | 1 + .../ConsumerGroups/Details/Details.tsx | 1 + .../Details/ResetOffsets/ResetOffsets.tsx | 1 + .../src/components/ConsumerGroups/List.tsx | 2 +- frontend/src/components/KsqlDb/KsqlDb.tsx | 2 +- .../Nav/ClusterMenu/ClusterMenu.tsx | 48 ++++++++----------- .../__tests__/ClusterMenu.spec.tsx | 9 ++-- frontend/src/components/Nav/Menu/MenuTab.tsx | 13 ++--- .../Nav/Menu/__tests__/MenuTab.spec.tsx | 2 +- frontend/src/components/Nav/Nav.styled.ts | 9 ++++ frontend/src/components/Nav/Nav.tsx | 16 +++++-- .../components/Schemas/Details/Details.tsx | 1 + frontend/src/components/Schemas/Diff/Diff.tsx | 1 + frontend/src/components/Schemas/Edit/Form.tsx | 1 + frontend/src/components/Schemas/List/List.tsx | 2 +- frontend/src/components/Schemas/New/New.tsx | 1 + .../src/components/Topics/List/ListPage.tsx | 6 ++- frontend/src/components/Topics/New/New.tsx | 1 + .../src/components/Topics/Topic/Topic.tsx | 1 + .../common/PageHeading/PageHeading.styled.ts | 19 ++++---- .../common/PageHeading/PageHeading.tsx | 19 +++++++- frontend/src/theme/theme.ts | 2 + 27 files changed, 102 insertions(+), 64 deletions(-) diff --git a/frontend/src/components/ACLPage/List/List.tsx b/frontend/src/components/ACLPage/List/List.tsx index 26155172b..93a8c1e77 100644 --- a/frontend/src/components/ACLPage/List/List.tsx +++ b/frontend/src/components/ACLPage/List/List.tsx @@ -149,7 +149,7 @@ const ACList: React.FC = () => { return ( - + { return ( <> { return ( <> - + { return (
{ return ( <> - + {!isReadOnly && ( { return ( {
{ return ( <> { return ( <> - + diff --git a/frontend/src/components/KsqlDb/KsqlDb.tsx b/frontend/src/components/KsqlDb/KsqlDb.tsx index d105720aa..6876a3685 100644 --- a/frontend/src/components/KsqlDb/KsqlDb.tsx +++ b/frontend/src/components/KsqlDb/KsqlDb.tsx @@ -29,7 +29,7 @@ const KsqlDb: React.FC = () => { return ( <> - + void; } const ClusterMenu: FC = ({ name, status, features, - singleMode, + openTab, + onTabClick, }) => { - const hasFeatureConfigured = (key: ClusterFeaturesEnum) => - features?.includes(key); - const [isOpen, setIsOpen] = useState(!!singleMode); const location = useLocation(); - const getIsMenuItemActive = (path: string) => - location.pathname.includes(path); + const getIsMenuItemActive = (path: string) => location.pathname === path; + + const hasFeatureConfigured = (key: ClusterFeaturesEnum) => + features?.includes(key); return (
    setIsOpen((prev) => !prev)} + isOpen={openTab === name} + onClick={() => onTabClick(name)} /> - {isOpen && ( + {hasFeatureConfigured(ClusterFeaturesEnum.SCHEMA_REGISTRY) && ( )} {hasFeatureConfigured(ClusterFeaturesEnum.KAFKA_CONNECT) && ( )} {hasFeatureConfigured(ClusterFeaturesEnum.KSQL_DB) && ( @@ -91,13 +85,13 @@ const ClusterMenu: FC = ({ {(hasFeatureConfigured(ClusterFeaturesEnum.KAFKA_ACL_VIEW) || hasFeatureConfigured(ClusterFeaturesEnum.KAFKA_ACL_EDIT)) && ( )} - )} +
); }; diff --git a/frontend/src/components/Nav/ClusterMenu/__tests__/ClusterMenu.spec.tsx b/frontend/src/components/Nav/ClusterMenu/__tests__/ClusterMenu.spec.tsx index ce0bb9731..e998b820e 100644 --- a/frontend/src/components/Nav/ClusterMenu/__tests__/ClusterMenu.spec.tsx +++ b/frontend/src/components/Nav/ClusterMenu/__tests__/ClusterMenu.spec.tsx @@ -8,12 +8,15 @@ import { render } from 'lib/testHelpers'; import { onlineClusterPayload } from 'lib/fixtures/clusters'; describe('ClusterMenu', () => { - const setupComponent = (cluster: Cluster, singleMode?: boolean) => ( + const handleTabClick = jest.fn(); + + const setupComponent = (cluster: Cluster, openTab?: string | false) => ( ); const getMenuItems = () => screen.getAllByRole('menuitem'); @@ -59,7 +62,7 @@ describe('ClusterMenu', () => { expect(screen.getByTitle('KSQL DB')).toBeInTheDocument(); }); it('renders open cluster menu', () => { - render(setupComponent(onlineClusterPayload, true), { + render(setupComponent(onlineClusterPayload), { initialEntries: [clusterConnectorsPath(onlineClusterPayload.name)], }); diff --git a/frontend/src/components/Nav/Menu/MenuTab.tsx b/frontend/src/components/Nav/Menu/MenuTab.tsx index 9214c3b00..36752bedd 100644 --- a/frontend/src/components/Nav/Menu/MenuTab.tsx +++ b/frontend/src/components/Nav/Menu/MenuTab.tsx @@ -1,4 +1,4 @@ -import React, { type FC } from 'react'; +import React, { FC } from 'react'; import { ServerStatus } from 'generated-sources'; import * as S from './styled'; @@ -7,16 +7,11 @@ export interface MenuTabProps { title: string; status: ServerStatus; isOpen: boolean; - toggleClusterMenu: () => void; + onClick: () => void; } -const MenuTab: FC = ({ - title, - toggleClusterMenu, - status, - isOpen, -}) => ( - +const MenuTab: FC = ({ title, status, isOpen, onClick }) => ( + diff --git a/frontend/src/components/Nav/Menu/__tests__/MenuTab.spec.tsx b/frontend/src/components/Nav/Menu/__tests__/MenuTab.spec.tsx index 50b4ba88e..d94abe03d 100644 --- a/frontend/src/components/Nav/Menu/__tests__/MenuTab.spec.tsx +++ b/frontend/src/components/Nav/Menu/__tests__/MenuTab.spec.tsx @@ -14,7 +14,7 @@ describe('MenuTab component', () => { status={ServerStatus.ONLINE} isOpen title={testClusterName} - toggleClusterMenu={toggleClusterMenuMock} + onClick={toggleClusterMenuMock} {...props} /> ); diff --git a/frontend/src/components/Nav/Nav.styled.ts b/frontend/src/components/Nav/Nav.styled.ts index 4f6bde408..1aef992b7 100644 --- a/frontend/src/components/Nav/Nav.styled.ts +++ b/frontend/src/components/Nav/Nav.styled.ts @@ -11,3 +11,12 @@ export const List = styled.ul.attrs({ role: 'menu' })` margin-bottom: 2px; } `; + +export const AccordionContent = styled.div<{ isOpen: boolean }>` + overflow: hidden; + max-height: ${({ isOpen }) => (isOpen ? '500px' : '0')}; + opacity: ${({ isOpen }) => (isOpen ? '1' : '0')}; + transition: + max-height 0.4s ease-out, + opacity 0.3s ease-out; +`; diff --git a/frontend/src/components/Nav/Nav.tsx b/frontend/src/components/Nav/Nav.tsx index 2741b4351..a9bbf45b1 100644 --- a/frontend/src/components/Nav/Nav.tsx +++ b/frontend/src/components/Nav/Nav.tsx @@ -1,26 +1,32 @@ +import React, { FC, useState } from 'react'; import { useClusters } from 'lib/hooks/api/clusters'; -import React, { type FC } from 'react'; import * as S from './Nav.styled'; import MenuItem from './Menu/MenuItem'; import ClusterMenu from './ClusterMenu/ClusterMenu'; const Nav: FC = () => { - const clusters = useClusters(); + const [openTab, setOpenTab] = useState(false); + const { isSuccess, data: clusters } = useClusters(); + + const handleTabChange = (tabName: string) => { + setOpenTab((prev) => (prev === tabName ? false : tabName)); + }; return ( diff --git a/frontend/src/components/Schemas/Details/Details.tsx b/frontend/src/components/Schemas/Details/Details.tsx index 2d0c345d7..e275226c2 100644 --- a/frontend/src/components/Schemas/Details/Details.tsx +++ b/frontend/src/components/Schemas/Details/Details.tsx @@ -72,6 +72,7 @@ const Details: React.FC = () => { return ( <> { return ( <> = ({ schema }) => { return ( { return ( <> - + {!isReadOnly && ( <> diff --git a/frontend/src/components/Schemas/New/New.tsx b/frontend/src/components/Schemas/New/New.tsx index 4bfcc4605..3f45b8364 100644 --- a/frontend/src/components/Schemas/New/New.tsx +++ b/frontend/src/components/Schemas/New/New.tsx @@ -72,6 +72,7 @@ const New: React.FC = () => { return ( { const { isReadOnly } = React.useContext(ClusterContext); + const { clusterName } = useAppParams(); const [searchParams, setSearchParams] = useSearchParams(); // Set the search params to the url based on the localStorage value @@ -46,7 +48,7 @@ const ListPage: React.FC = () => { return ( <> - + {!isReadOnly && ( { return ( <> { return ( <> theme.pageHeading.backLink.color.disabled}; + position: relative; +`; + export const BackLink = styled(NavLink)` color: ${({ theme }) => theme.pageHeading.backLink.color.normal}; position: relative; @@ -13,16 +18,12 @@ export const BackLink = styled(NavLink)` &:hover { ${({ theme }) => theme.pageHeading.backLink.color.hover}; } +`; - &::after { - content: ''; - position: absolute; - right: -11px; - bottom: 2px; - border-left: 1px solid ${({ theme }) => theme.pageHeading.dividerColor}; - height: 20px; - transform: rotate(14deg); - } +export const Slash = styled.text` + color: ${({ theme }) => theme.pageHeading.backLink.color.disabled}; + position: relative; + margin: 0 8px; `; export const Wrapper = styled.div` diff --git a/frontend/src/components/common/PageHeading/PageHeading.tsx b/frontend/src/components/common/PageHeading/PageHeading.tsx index ffbdc1a27..56624254c 100644 --- a/frontend/src/components/common/PageHeading/PageHeading.tsx +++ b/frontend/src/components/common/PageHeading/PageHeading.tsx @@ -5,12 +5,14 @@ import * as S from './PageHeading.styled'; interface PageHeadingProps { text: string; + clusterName?: string; backTo?: string; backText?: string; } const PageHeading: React.FC> = ({ text, + clusterName, backTo, backText, children, @@ -20,8 +22,21 @@ const PageHeading: React.FC> = ({ return ( - {isBackButtonVisible && {backText}} - {text} + + {clusterName && ( + <> + {clusterName} + / + + )} + {isBackButtonVisible && ( + <> + {backText} + / + + )} + {text} +
{children}
 diff --git a/frontend/src/theme/theme.ts b/frontend/src/theme/theme.ts index f6cd2bacc..6c1d9cee5 100644 --- a/frontend/src/theme/theme.ts +++ b/frontend/src/theme/theme.ts @@ -371,6 +371,7 @@ export const theme = { dividerColor: Colors.neutral[30], backLink: { color: { + disabled: Colors.neutral[50], normal: Colors.brand[70], hover: Colors.brand[60], }, @@ -855,6 +856,7 @@ export const darkTheme: ThemeType = { dividerColor: Colors.neutral[50], backLink: { color: { + disabled: Colors.neutral[50], normal: Colors.brand[30], hover: Colors.brand[15], }, From b8a9dabe10e5fcf995e096b1b84363d4bcea3132 Mon Sep 17 00:00:00 2001 From: Alexis Demetriou Date: Thu, 22 Aug 2024 11:35:58 +0300 Subject: [PATCH 2/5] Fixed active full path Kafka instance --- frontend/src/components/Nav/ClusterMenu/ClusterMenu.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/src/components/Nav/ClusterMenu/ClusterMenu.tsx b/frontend/src/components/Nav/ClusterMenu/ClusterMenu.tsx index 4d492256d..41ad5d4c6 100644 --- a/frontend/src/components/Nav/ClusterMenu/ClusterMenu.tsx +++ b/frontend/src/components/Nav/ClusterMenu/ClusterMenu.tsx @@ -31,7 +31,7 @@ const ClusterMenu: FC = ({ }) => { const location = useLocation(); - const getIsMenuItemActive = (path: string) => location.pathname === path; + const getIsMenuItemActive = (path: string) => location.pathname.includes(path); const hasFeatureConfigured = (key: ClusterFeaturesEnum) => features?.includes(key); From 431cb74eddd23ec427e6308f496a395b5dd55ac9 Mon Sep 17 00:00:00 2001 From: Alexis Demetriou Date: Thu, 22 Aug 2024 11:36:45 +0300 Subject: [PATCH 3/5] Fixed active full path Kafka instance --- frontend/src/components/Nav/ClusterMenu/ClusterMenu.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/frontend/src/components/Nav/ClusterMenu/ClusterMenu.tsx b/frontend/src/components/Nav/ClusterMenu/ClusterMenu.tsx index 41ad5d4c6..921dfed17 100644 --- a/frontend/src/components/Nav/ClusterMenu/ClusterMenu.tsx +++ b/frontend/src/components/Nav/ClusterMenu/ClusterMenu.tsx @@ -31,7 +31,8 @@ const ClusterMenu: FC = ({ }) => { const location = useLocation(); - const getIsMenuItemActive = (path: string) => location.pathname.includes(path); + const getIsMenuItemActive = (path: string) => + location.pathname.includes(path); const hasFeatureConfigured = (key: ClusterFeaturesEnum) => features?.includes(key); From b52f5b91fc435dff7cb42b91e8bad24b75e595cd Mon Sep 17 00:00:00 2001 From: Alexis Demetriou Date: Mon, 16 Sep 2024 13:21:34 +0300 Subject: [PATCH 4/5] Fixed tests errors --- .../Connect/List/__tests__/ListPage.spec.tsx | 4 ++-- .../Nav/ClusterMenu/__tests__/ClusterMenu.spec.tsx | 7 ------- frontend/src/components/Nav/__tests__/Nav.spec.tsx | 4 ++-- .../src/components/Topics/New/__test__/New.spec.tsx | 12 ++++++------ 4 files changed, 10 insertions(+), 17 deletions(-) diff --git a/frontend/src/components/Connect/List/__tests__/ListPage.spec.tsx b/frontend/src/components/Connect/List/__tests__/ListPage.spec.tsx index 0cbe4dd78..2e19eaa63 100644 --- a/frontend/src/components/Connect/List/__tests__/ListPage.spec.tsx +++ b/frontend/src/components/Connect/List/__tests__/ListPage.spec.tsx @@ -49,7 +49,7 @@ describe('Connectors List Page', () => { it('renders header without create button for readonly cluster', async () => { await renderComponent({ ...initialValue, isReadOnly: true }); expect( - screen.getByRole('heading', { name: 'Connectors' }) + screen.getByRole('heading', { name: /local \/ Connectors/ }) ).toBeInTheDocument(); expect( screen.queryByRole('link', { name: 'Create Connector' }) @@ -59,7 +59,7 @@ describe('Connectors List Page', () => { it('renders header with create button for read/write cluster', async () => { await renderComponent(); expect( - screen.getByRole('heading', { name: 'Connectors' }) + screen.getByRole('heading', { name: /local \/ Connectors/ }) ).toBeInTheDocument(); expect( screen.getByRole('link', { name: 'Create Connector' }) diff --git a/frontend/src/components/Nav/ClusterMenu/__tests__/ClusterMenu.spec.tsx b/frontend/src/components/Nav/ClusterMenu/__tests__/ClusterMenu.spec.tsx index e998b820e..5d536fabe 100644 --- a/frontend/src/components/Nav/ClusterMenu/__tests__/ClusterMenu.spec.tsx +++ b/frontend/src/components/Nav/ClusterMenu/__tests__/ClusterMenu.spec.tsx @@ -2,7 +2,6 @@ import React from 'react'; import { screen } from '@testing-library/react'; import { Cluster, ClusterFeaturesEnum } from 'generated-sources'; import ClusterMenu from 'components/Nav/ClusterMenu/ClusterMenu'; -import userEvent from '@testing-library/user-event'; import { clusterConnectorsPath } from 'lib/paths'; import { render } from 'lib/testHelpers'; import { onlineClusterPayload } from 'lib/fixtures/clusters'; @@ -31,8 +30,6 @@ describe('ClusterMenu', () => { render(setupComponent(onlineClusterPayload)); expect(getCluster()).toBeInTheDocument(); - expect(getMenuItems().length).toEqual(1); - await userEvent.click(getMenuItem()); expect(getMenuItems().length).toEqual(4); expect(getBrokers()).toBeInTheDocument(); @@ -50,8 +47,6 @@ describe('ClusterMenu', () => { ], }) ); - expect(getMenuItems().length).toEqual(1); - await userEvent.click(getMenuItem()); expect(getMenuItems().length).toEqual(7); expect(getBrokers()).toBeInTheDocument(); @@ -80,8 +75,6 @@ describe('ClusterMenu', () => { }), { initialEntries: [clusterConnectorsPath(onlineClusterPayload.name)] } ); - expect(getMenuItems().length).toEqual(1); - await userEvent.click(getMenuItem()); expect(getMenuItems().length).toEqual(5); const kafkaConnect = getKafkaConnect(); diff --git a/frontend/src/components/Nav/__tests__/Nav.spec.tsx b/frontend/src/components/Nav/__tests__/Nav.spec.tsx index 582c34141..5ce170093 100644 --- a/frontend/src/components/Nav/__tests__/Nav.spec.tsx +++ b/frontend/src/components/Nav/__tests__/Nav.spec.tsx @@ -34,8 +34,8 @@ describe('Nav', () => { it('renders ClusterMenu', () => { renderComponent([onlineClusterPayload, offlineClusterPayload]); - expect(screen.getAllByRole('menu').length).toEqual(3); - expect(getMenuItemsCount()).toEqual(3); + expect(screen.getAllByRole('menu').length).toEqual(5); + expect(getMenuItemsCount()).toEqual(9); expect(getDashboard()).toBeInTheDocument(); expect(screen.getByText(onlineClusterPayload.name)).toBeInTheDocument(); expect(screen.getByText(offlineClusterPayload.name)).toBeInTheDocument(); diff --git a/frontend/src/components/Topics/New/__test__/New.spec.tsx b/frontend/src/components/Topics/New/__test__/New.spec.tsx index 867291e0b..d5ae67744 100644 --- a/frontend/src/components/Topics/New/__test__/New.spec.tsx +++ b/frontend/src/components/Topics/New/__test__/New.spec.tsx @@ -42,24 +42,24 @@ describe('New', () => { })); }); it('checks header for create new', async () => { - await act(() => { + await act(async () => { renderComponent(clusterTopicNewPath(clusterName)); }); - expect(screen.getByRole('heading', { name: 'Create' })).toBeInTheDocument(); + expect(screen.getByRole('heading', { name: /local \/ Topics \/ Create/ })).toBeInTheDocument(); }); it('checks header for copy', async () => { - await act(() => { + await act(async () => { renderComponent(`${clusterTopicCopyPath(clusterName)}?name=test`); }); - expect(screen.getByRole('heading', { name: 'Copy' })).toBeInTheDocument(); + expect(screen.getByRole('heading', { name: /local \/ Topics \/ Copy/ })).toBeInTheDocument(); }); it('validates form', async () => { renderComponent(clusterTopicNewPath(clusterName)); await userEvent.type(screen.getByPlaceholderText('Topic Name'), topicName); await userEvent.clear(screen.getByPlaceholderText('Topic Name')); await userEvent.tab(); - await expect( + expect( screen.getByText('Topic Name is required') ).toBeInTheDocument(); await userEvent.type( @@ -68,7 +68,7 @@ describe('New', () => { ); await userEvent.clear(screen.getByLabelText('Number of Partitions *')); await userEvent.tab(); - await expect( + expect( screen.getByText('Number of Partitions is required and must be a number') ).toBeInTheDocument(); From 0135b61d5629a710a9ac4ef5adb7a2023c643573 Mon Sep 17 00:00:00 2001 From: Alexis Demetriou Date: Mon, 16 Sep 2024 13:35:44 +0300 Subject: [PATCH 5/5] Fixed isuues --- .../Nav/ClusterMenu/__tests__/ClusterMenu.spec.tsx | 1 - .../src/components/Topics/New/__test__/New.spec.tsx | 12 +++++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/frontend/src/components/Nav/ClusterMenu/__tests__/ClusterMenu.spec.tsx b/frontend/src/components/Nav/ClusterMenu/__tests__/ClusterMenu.spec.tsx index 5d536fabe..2da2202c8 100644 --- a/frontend/src/components/Nav/ClusterMenu/__tests__/ClusterMenu.spec.tsx +++ b/frontend/src/components/Nav/ClusterMenu/__tests__/ClusterMenu.spec.tsx @@ -19,7 +19,6 @@ describe('ClusterMenu', () => { /> ); const getMenuItems = () => screen.getAllByRole('menuitem'); - const getMenuItem = () => screen.getByRole('menuitem'); const getBrokers = () => screen.getByTitle('Brokers'); const getTopics = () => screen.getByTitle('Brokers'); const getConsumers = () => screen.getByTitle('Brokers'); diff --git a/frontend/src/components/Topics/New/__test__/New.spec.tsx b/frontend/src/components/Topics/New/__test__/New.spec.tsx index d5ae67744..2793840fc 100644 --- a/frontend/src/components/Topics/New/__test__/New.spec.tsx +++ b/frontend/src/components/Topics/New/__test__/New.spec.tsx @@ -45,23 +45,25 @@ describe('New', () => { await act(async () => { renderComponent(clusterTopicNewPath(clusterName)); }); - expect(screen.getByRole('heading', { name: /local \/ Topics \/ Create/ })).toBeInTheDocument(); + expect( + screen.getByRole('heading', { name: /local \/ Topics \/ Create/ }) + ).toBeInTheDocument(); }); it('checks header for copy', async () => { await act(async () => { renderComponent(`${clusterTopicCopyPath(clusterName)}?name=test`); }); - expect(screen.getByRole('heading', { name: /local \/ Topics \/ Copy/ })).toBeInTheDocument(); + expect( + screen.getByRole('heading', { name: /local \/ Topics \/ Copy/ }) + ).toBeInTheDocument(); }); it('validates form', async () => { renderComponent(clusterTopicNewPath(clusterName)); await userEvent.type(screen.getByPlaceholderText('Topic Name'), topicName); await userEvent.clear(screen.getByPlaceholderText('Topic Name')); await userEvent.tab(); - expect( - screen.getByText('Topic Name is required') - ).toBeInTheDocument(); + expect(screen.getByText('Topic Name is required')).toBeInTheDocument(); await userEvent.type( screen.getByLabelText('Number of Partitions *'), minValue