From 0a40f13580e9b6af226ab6bdbd49e2505d988a2c Mon Sep 17 00:00:00 2001 From: blam Date: Thu, 18 Jul 2024 13:05:32 +0200 Subject: [PATCH 1/4] chore: wip Signed-off-by: blam --- .../src/components/TabbedLayout/RoutedTabs.tsx | 3 +++ .../src/layout/HeaderTabs/HeaderTabs.test.tsx | 9 --------- .../core-components/src/layout/HeaderTabs/HeaderTabs.tsx | 7 +------ 3 files changed, 4 insertions(+), 15 deletions(-) diff --git a/packages/core-components/src/components/TabbedLayout/RoutedTabs.tsx b/packages/core-components/src/components/TabbedLayout/RoutedTabs.tsx index 2c727efe1b..6a5bf2970f 100644 --- a/packages/core-components/src/components/TabbedLayout/RoutedTabs.tsx +++ b/packages/core-components/src/components/TabbedLayout/RoutedTabs.tsx @@ -96,6 +96,9 @@ export function RoutedTabs(props: { routes: SubRoute[] }) { tabs={headerTabs} selectedIndex={index} onChange={onTabChange} + tabProps={{ + component: Link, + }} /> diff --git a/packages/core-components/src/layout/HeaderTabs/HeaderTabs.test.tsx b/packages/core-components/src/layout/HeaderTabs/HeaderTabs.test.tsx index dad40c4615..5aec83bc2b 100644 --- a/packages/core-components/src/layout/HeaderTabs/HeaderTabs.test.tsx +++ b/packages/core-components/src/layout/HeaderTabs/HeaderTabs.test.tsx @@ -95,13 +95,4 @@ describe('', () => { await user.click(rendered.getByText('Docs')); expect(mockOnChange).toHaveBeenCalledTimes(1); }); - - it('should render 2 nav tabs', async () => { - const rendered = await renderInTestApp(); - const tabs = rendered.queryAllByTestId(id => id.startsWith('header-tab')); - expect(tabs).toHaveLength(2); - tabs.forEach(tab => { - expect(tab.tagName.toLocaleLowerCase('en-US')).toBe('a'); - }); - }); }); diff --git a/packages/core-components/src/layout/HeaderTabs/HeaderTabs.tsx b/packages/core-components/src/layout/HeaderTabs/HeaderTabs.tsx index 9ee9c5525f..002c9fe1ea 100644 --- a/packages/core-components/src/layout/HeaderTabs/HeaderTabs.tsx +++ b/packages/core-components/src/layout/HeaderTabs/HeaderTabs.tsx @@ -18,7 +18,6 @@ import { makeStyles } from '@material-ui/core/styles'; import TabUI, { TabProps } from '@material-ui/core/Tab'; import Tabs from '@material-ui/core/Tabs'; import React, { useCallback, useEffect, useState } from 'react'; -import { Link } from '../../components/Link'; // TODO(blam): Remove this implementation when the Tabs are ready // This is just a temporary solution to implementing tabs for now @@ -96,9 +95,7 @@ export function HeaderTabs(props: HeaderTabsProps) { setSelectedTab(selectedIndex); } }, [selectedIndex]); - function removeLeadingSlash(path: string) { - return path.replace(/^\//, ''); - } + return ( Date: Fri, 19 Jul 2024 14:06:17 +0200 Subject: [PATCH 2/4] chore: attempt to fix the tabs Signed-off-by: blam --- .../TabbedLayout/RoutedTabs.test.tsx | 12 +++++ .../components/TabbedLayout/RoutedTabs.tsx | 51 ++++++++----------- 2 files changed, 33 insertions(+), 30 deletions(-) diff --git a/packages/core-components/src/components/TabbedLayout/RoutedTabs.test.tsx b/packages/core-components/src/components/TabbedLayout/RoutedTabs.test.tsx index ec91208eee..991276c5d3 100644 --- a/packages/core-components/src/components/TabbedLayout/RoutedTabs.test.tsx +++ b/packages/core-components/src/components/TabbedLayout/RoutedTabs.test.tsx @@ -160,4 +160,16 @@ describe('RoutedTabs', () => { rendered.queryByText('tabbed-test-content-2'), ).not.toBeInTheDocument(); }); + + it('should render the tabs as links', async () => { + const routes = [testRoute2, testRoute1, testRoute3]; + const rendered = await renderInTestApp(); + + const tabs = rendered.queryAllByRole('tab'); + + for (const [k, v] of Object.entries(tabs)) { + expect(v.tagName).toBe('A'); + expect(v).toHaveAttribute('href', routes[Number(k)].path); + } + }); }); diff --git a/packages/core-components/src/components/TabbedLayout/RoutedTabs.tsx b/packages/core-components/src/components/TabbedLayout/RoutedTabs.tsx index 6a5bf2970f..d6f6eab144 100644 --- a/packages/core-components/src/components/TabbedLayout/RoutedTabs.tsx +++ b/packages/core-components/src/components/TabbedLayout/RoutedTabs.tsx @@ -15,15 +15,11 @@ */ import React, { useMemo } from 'react'; import { Helmet } from 'react-helmet'; -import { - matchRoutes, - useNavigate, - useParams, - useRoutes, -} from 'react-router-dom'; +import { matchRoutes, useParams, useRoutes } from 'react-router-dom'; import { Content } from '../../layout/Content'; import { HeaderTabs } from '../../layout/HeaderTabs'; import { SubRoute } from './types'; +import { Link } from '../Link'; export function useSelectedSubRoute(subRoutes: SubRoute[]): { index: number; @@ -68,38 +64,33 @@ export function useSelectedSubRoute(subRoutes: SubRoute[]): { export function RoutedTabs(props: { routes: SubRoute[] }) { const { routes } = props; - const navigate = useNavigate(); + const { index, route, element } = useSelectedSubRoute(routes); const headerTabs = useMemo( () => - routes.map(t => ({ - id: t.path, - label: t.title, - tabProps: t.tabProps, - })), + routes.map(t => { + const { path, title, tabProps } = t; + let to = path; + // Remove trailing /* + to = to.replace(/\/\*$/, ''); + // And remove leading / for relative navigation + to = to.replace(/^\//, ''); + return { + id: path, + label: title, + tabProps: { + ...tabProps, + component: Link, + to, + }, + }; + }), [routes], ); - const onTabChange = (tabIndex: number) => { - let { path } = routes[tabIndex]; - // Remove trailing /* - path = path.replace(/\/\*$/, ''); - // And remove leading / for relative navigation - path = path.replace(/^\//, ''); - // Note! route resolves relative to the position in the React tree, - // not relative to current location - navigate(path); - }; return ( <> - + {element} From 678971ad41380d5b958ed4b9f72e9372385d9380 Mon Sep 17 00:00:00 2001 From: blam Date: Fri, 19 Jul 2024 14:07:17 +0200 Subject: [PATCH 3/4] chore: added changeset Signed-off-by: blam --- .changeset/eighty-mirrors-flow.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/eighty-mirrors-flow.md diff --git a/.changeset/eighty-mirrors-flow.md b/.changeset/eighty-mirrors-flow.md new file mode 100644 index 0000000000..57e978a78c --- /dev/null +++ b/.changeset/eighty-mirrors-flow.md @@ -0,0 +1,5 @@ +--- +'@backstage/core-components': patch +--- + +Move the `Link` component to the `RoutedTabs` instead of the `HeaderTabs` component From d4e33aca1adc461063130fa36e255498490785ec Mon Sep 17 00:00:00 2001 From: blam Date: Tue, 23 Jul 2024 08:19:53 +0200 Subject: [PATCH 4/4] chore: fixing tests Signed-off-by: blam --- .../src/components/TabbedLayout/RoutedTabs.test.tsx | 5 +++-- .../src/components/TabbedLayout/RoutedTabs.tsx | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/core-components/src/components/TabbedLayout/RoutedTabs.test.tsx b/packages/core-components/src/components/TabbedLayout/RoutedTabs.test.tsx index 991276c5d3..754ac2ec8d 100644 --- a/packages/core-components/src/components/TabbedLayout/RoutedTabs.test.tsx +++ b/packages/core-components/src/components/TabbedLayout/RoutedTabs.test.tsx @@ -162,14 +162,15 @@ describe('RoutedTabs', () => { }); it('should render the tabs as links', async () => { - const routes = [testRoute2, testRoute1, testRoute3]; + const routes = [testRoute1, testRoute2, testRoute3]; + const expectedHrefs = ['/', '/some-other-path', '/some-other-path-similar']; const rendered = await renderInTestApp(); const tabs = rendered.queryAllByRole('tab'); for (const [k, v] of Object.entries(tabs)) { expect(v.tagName).toBe('A'); - expect(v).toHaveAttribute('href', routes[Number(k)].path); + expect(v).toHaveAttribute('href', expectedHrefs[Number(k)]); } }); }); diff --git a/packages/core-components/src/components/TabbedLayout/RoutedTabs.tsx b/packages/core-components/src/components/TabbedLayout/RoutedTabs.tsx index d6f6eab144..b57a3782bb 100644 --- a/packages/core-components/src/components/TabbedLayout/RoutedTabs.tsx +++ b/packages/core-components/src/components/TabbedLayout/RoutedTabs.tsx @@ -79,9 +79,9 @@ export function RoutedTabs(props: { routes: SubRoute[] }) { id: path, label: title, tabProps: { - ...tabProps, component: Link, to, + ...tabProps, }, }; }),