From b66ae2b542434c12d80430b90c738addf98c428f Mon Sep 17 00:00:00 2001 From: Yousif Al-Raheem Date: Wed, 16 Mar 2022 11:44:40 +0100 Subject: [PATCH] use outlet to render extra tabs instead of tabs prop Signed-off-by: Yousif Al-Raheem --- packages/app/src/App.tsx | 17 +++---- plugins/user-settings/README.md | 25 +++++------ .../src/components/SettingsPage.test.tsx | 45 ++++++++++++++++--- .../src/components/SettingsPage.tsx | 35 +++++++++------ 4 files changed, 78 insertions(+), 44 deletions(-) diff --git a/packages/app/src/App.tsx b/packages/app/src/App.tsx index 9151e55e51..b3bb511c90 100644 --- a/packages/app/src/App.tsx +++ b/packages/app/src/App.tsx @@ -69,7 +69,8 @@ import { techdocsPlugin, TechDocsReaderPage, } from '@backstage/plugin-techdocs'; -import { UserSettingsPage, SettingsTab } from '@backstage/plugin-user-settings'; +import { UserSettingsPage } from '@backstage/plugin-user-settings'; +import { AdvancedSettings } from './components/advancedSettings'; import AlarmIcon from '@material-ui/icons/Alarm'; import React from 'react'; import { hot } from 'react-hot-loader/root'; @@ -82,7 +83,6 @@ import { LowerCaseValuePickerFieldExtension } from './components/scaffolder/cust import { searchPage } from './components/search/SearchPage'; import { providers } from './identityProviders'; import * as plugins from './plugins'; -import { AdvancedSettings } from './components/advancedSettings'; import { techDocsPage } from './components/techdocs/TechDocsPage'; import { ApacheAirflowPage } from '@backstage/plugin-apache-airflow'; import { PermissionedRoute } from '@backstage/plugin-permission-react'; @@ -127,10 +127,6 @@ const app = createApp({ const AppProvider = app.getProvider(); const AppRouter = app.getRouter(); -const extraSettingTabs: SettingsTab[] = [ - { title: 'Advanced', content: }, -]; - const routes = ( @@ -219,10 +215,11 @@ const routes = ( path="/cost-insights/labeling-jobs" element={} /> - } - /> + }> + }> + Advanced + + } /> } /> diff --git a/plugins/user-settings/README.md b/plugins/user-settings/README.md index 71440ebfaf..689b11880d 100644 --- a/plugins/user-settings/README.md +++ b/plugins/user-settings/README.md @@ -68,23 +68,18 @@ const AppRoutes = () => ( By default, the plugin renders 3 tabs of settings; GENERAL, AUTHENTICATION PROVIDERS, and FEATURE FLAGS. -If you want to add more options for your users, use the `tabs` prop: +If you want to add more options for your users, +just pass the extra tabs as children of the `UserSettingsPage` route +where the children of that route represents the title of the tab. +The path is in this case a child of the settings path, +in the example below it would be `/settings/advanced` so that you can easily link to it. ```tsx -import { UserSettingsPage, SettingsTab } from '@backstage/plugin-user-settings'; - -const extraSettingTabs: SettingsTab[] = [ - { title: 'Advanced', content: }, -]; - -const AppRoutes = () => ( - - } - /> - -); +}> + }> + Advanced + + ``` To standardize the UI of all setting tabs, diff --git a/plugins/user-settings/src/components/SettingsPage.test.tsx b/plugins/user-settings/src/components/SettingsPage.test.tsx index 846f120a6b..bee395a2fa 100644 --- a/plugins/user-settings/src/components/SettingsPage.test.tsx +++ b/plugins/user-settings/src/components/SettingsPage.test.tsx @@ -16,7 +16,14 @@ import React from 'react'; import { renderWithEffects, wrapInTestApp } from '@backstage/test-utils'; -import { SettingsPage, SettingsTab } from './SettingsPage'; +import { SettingsPage } from './SettingsPage'; + +jest.mock('react-router', () => ({ + ...jest.requireActual('react-router'), + useOutlet: jest.fn().mockReturnValue(undefined), +})); + +import { useOutlet, Route } from 'react-router'; describe('', () => { it('should render the settings page with 3 tabs', async () => { @@ -29,15 +36,41 @@ describe('', () => { }); it('should render the settings page with 4 tabs when extra tabs are provided', async () => { - const extraTabs: SettingsTab[] = [ - { title: 'Advanced', content:
advanced content
}, - ]; + const advancedTabRoute = ( + <> + Advanced settings}> + Advanced + + + ); + (useOutlet as jest.Mock).mockReturnValueOnce(advancedTabRoute); const { container } = await renderWithEffects( - wrapInTestApp(), + wrapInTestApp(), ); const tabs = container.querySelectorAll('[class*=MuiTabs-root] button'); expect(tabs).toHaveLength(4); - expect(tabs[3].textContent).toEqual(extraTabs[0].title); + expect(tabs[3].textContent).toEqual('Advanced'); + }); + + it('should throw error if non compliant route is passed to settings routes', async () => { + const advancedTabRoute = ( + <> + Advanced settings}> + Advanced + +
Non compliant element
+ + ); + (useOutlet as jest.Mock).mockReturnValueOnce(advancedTabRoute); + let error: Error; + try { + await renderWithEffects(wrapInTestApp()); + } catch (err) { + error = err; + } + expect(error!.message).toEqual( + expect.stringContaining('Invalid element passed'), + ); }); }); diff --git a/plugins/user-settings/src/components/SettingsPage.tsx b/plugins/user-settings/src/components/SettingsPage.tsx index a8ef1bda8f..0e21241ddf 100644 --- a/plugins/user-settings/src/components/SettingsPage.tsx +++ b/plugins/user-settings/src/components/SettingsPage.tsx @@ -20,23 +20,30 @@ import { SidebarPinStateContext, TabbedLayout, } from '@backstage/core-components'; -import React, { useContext } from 'react'; +import React, { useContext, ReactElement } from 'react'; +import { useOutlet } from 'react-router'; import { UserSettingsAuthProviders } from './AuthProviders'; import { UserSettingsFeatureFlags } from './FeatureFlags'; import { UserSettingsGeneral } from './General'; -export interface SettingsTab { - title: string; - content: React.ReactElement; -} - type Props = { providerSettings?: JSX.Element; - tabs?: SettingsTab[]; }; -export const SettingsPage = ({ providerSettings, tabs }: Props) => { +export const SettingsPage = ({ providerSettings }: Props) => { const { isMobile } = useContext(SidebarPinStateContext); + const outlet = useOutlet(); + + const extraTabs = (React.Children.toArray(outlet?.props?.children) || + []) as Array; + const notCompliantTab = Array.from(extraTabs).some( + child => (child.type as any)?.displayName !== 'Route', + ); + if (notCompliantTab) { + throw new Error( + 'Invalid element passed to SettingsPage Outlet. You may only pass children of type Route.', + ); + } return ( @@ -54,12 +61,14 @@ export const SettingsPage = ({ providerSettings, tabs }: Props) => { - {tabs?.map(({ title, content }) => { - // Hyphenated the title - const path = title.toLocaleLowerCase().replace(/\s/, '-'); + + {extraTabs.map((child, i) => { + const path: string = child.props.path; + const title: string = child.props.children; + return ( - - {content} + + {child} ); })}