From 3533075dfd48a02ce386f69e35391baeb180eb83 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Fri, 20 May 2022 13:00:51 +0200 Subject: [PATCH 1/7] Replace SidebarContext with versioned provider and hook. Signed-off-by: Eric Peterson --- packages/app/src/components/Root/Root.tsx | 6 +- packages/core-components/api-report.md | 16 ++-- packages/core-components/package.json | 1 + .../src/layout/Sidebar/Bar.tsx | 6 +- .../src/layout/Sidebar/Intro.tsx | 4 +- .../src/layout/Sidebar/Items.tsx | 6 +- .../src/layout/Sidebar/MobileSidebar.tsx | 7 +- .../layout/Sidebar/SidebarContext.test.tsx | 68 ++++++++++++++++ .../src/layout/Sidebar/SidebarContext.tsx | 78 +++++++++++++++++++ .../src/layout/Sidebar/SidebarSubmenu.tsx | 4 +- .../src/layout/Sidebar/config.ts | 16 ---- .../src/layout/Sidebar/index.ts | 14 +--- .../packages/app/src/components/Root/Root.tsx | 6 +- .../src/components/Root/Root.tsx | 6 +- plugins/shortcuts/src/ShortcutItem.test.tsx | 6 +- plugins/shortcuts/src/Shortcuts.test.tsx | 6 +- 16 files changed, 190 insertions(+), 60 deletions(-) create mode 100644 packages/core-components/src/layout/Sidebar/SidebarContext.test.tsx create mode 100644 packages/core-components/src/layout/Sidebar/SidebarContext.tsx diff --git a/packages/app/src/components/Root/Root.tsx b/packages/app/src/components/Root/Root.tsx index ac62c81899..e90fe29b54 100644 --- a/packages/app/src/components/Root/Root.tsx +++ b/packages/app/src/components/Root/Root.tsx @@ -14,7 +14,7 @@ * limitations under the License. */ -import React, { useContext, PropsWithChildren } from 'react'; +import React, { PropsWithChildren } from 'react'; import { Link, makeStyles } from '@material-ui/core'; import HomeIcon from '@material-ui/icons/Home'; import ExtensionIcon from '@material-ui/icons/Extension'; @@ -39,13 +39,13 @@ import { Shortcuts } from '@backstage/plugin-shortcuts'; import { Sidebar, sidebarConfig, - SidebarContext, SidebarDivider, SidebarGroup, SidebarItem, SidebarPage, SidebarScrollWrapper, SidebarSpace, + useSidebar, } from '@backstage/core-components'; import { MyGroupsSidebarItem } from '@backstage/plugin-org'; import GroupIcon from '@material-ui/icons/People'; @@ -68,7 +68,7 @@ const useSidebarLogoStyles = makeStyles({ const SidebarLogo = () => { const classes = useSidebarLogoStyles(); - const { isOpen } = useContext(SidebarContext); + const { isOpen } = useSidebar(); return (
diff --git a/packages/core-components/api-report.md b/packages/core-components/api-report.md index 727a50926c..e7ccc8e72b 100644 --- a/packages/core-components/api-report.md +++ b/packages/core-components/api-report.md @@ -16,7 +16,6 @@ import { CardHeaderProps } from '@material-ui/core/CardHeader'; import { Column } from '@material-table/core'; import { ComponentClass } from 'react'; import { ComponentProps } from 'react'; -import { Context } from 'react'; import { default as CSS_2 } from 'csstype'; import { CSSProperties } from 'react'; import { ElementType } from 'react'; @@ -903,13 +902,15 @@ export const sidebarConfig: { mobileSidebarHeight: number; }; -// Warning: (ae-missing-release-tag) "SidebarContext" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) -// // @public -export const SidebarContext: Context; +export const SidebarContextProvider: ({ + children, + value, +}: { + children: ReactNode; + value: SidebarContextType; +}) => JSX.Element; -// Warning: (ae-missing-release-tag) "SidebarContextType" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) -// // @public export type SidebarContextType = { isOpen: boolean; @@ -1447,6 +1448,9 @@ export class UserIdentity implements IdentityApi { signOut(): Promise; } +// @public +export const useSidebar: () => SidebarContextType; + // Warning: (ae-missing-release-tag) "useSupportConfig" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) // // @public (undocumented) diff --git a/packages/core-components/package.json b/packages/core-components/package.json index a988d8213e..6c7424af6e 100644 --- a/packages/core-components/package.json +++ b/packages/core-components/package.json @@ -37,6 +37,7 @@ "@backstage/core-plugin-api": "^1.0.2", "@backstage/errors": "^1.0.0", "@backstage/theme": "^0.2.15", + "@backstage/version-bridge": "^1.0.1", "@material-table/core": "^3.1.0", "@material-ui/core": "^4.12.2", "@material-ui/icons": "^4.9.1", diff --git a/packages/core-components/src/layout/Sidebar/Bar.tsx b/packages/core-components/src/layout/Sidebar/Bar.tsx index e78706bd62..2fdb2012dc 100644 --- a/packages/core-components/src/layout/Sidebar/Bar.tsx +++ b/packages/core-components/src/layout/Sidebar/Bar.tsx @@ -25,7 +25,6 @@ import { makeSidebarConfig, makeSidebarSubmenuConfig, SidebarConfig, - SidebarContext, SidebarConfigContext, SubmenuConfig, SidebarOptions, @@ -33,6 +32,7 @@ import { } from './config'; import { BackstageTheme } from '@backstage/theme'; import { SidebarPinStateContext, useContent } from './Page'; +import { SidebarContextProvider } from './SidebarContext'; import { MobileSidebar } from './MobileSidebar'; /** @public */ @@ -191,7 +191,7 @@ const DesktopSidebar = (props: DesktopSidebarProps) => { return (
- + ); }; diff --git a/packages/core-components/src/layout/Sidebar/Intro.tsx b/packages/core-components/src/layout/Sidebar/Intro.tsx index d1d8f4b2a2..51d453b22d 100644 --- a/packages/core-components/src/layout/Sidebar/Intro.tsx +++ b/packages/core-components/src/layout/Sidebar/Intro.tsx @@ -25,10 +25,10 @@ import { useLocalStorageValue } from '@react-hookz/web'; import { SidebarConfigContext, SidebarConfig, - SidebarContext, SIDEBAR_INTRO_LOCAL_STORAGE, } from './config'; import { SidebarDivider } from './Items'; +import { useSidebar } from './SidebarContext'; /** @public */ export type SidebarIntroClassKey = @@ -151,7 +151,7 @@ const recentlyViewedIntroText = 'And your recently viewed plugins will pop up here!'; export function SidebarIntro(_props: {}) { - const { isOpen } = useContext(SidebarContext); + const { isOpen } = useSidebar(); const defaultValue = { starredItemsDismissed: false, recentlyViewedItemsDismissed: false, diff --git a/packages/core-components/src/layout/Sidebar/Items.tsx b/packages/core-components/src/layout/Sidebar/Items.tsx index fde3c85c96..3d294a54a8 100644 --- a/packages/core-components/src/layout/Sidebar/Items.tsx +++ b/packages/core-components/src/layout/Sidebar/Items.tsx @@ -48,7 +48,6 @@ import { useResolvedPath, } from 'react-router-dom'; import { - SidebarContext, SidebarConfigContext, SidebarItemWithSubmenuContext, SidebarConfig, @@ -62,6 +61,7 @@ import DoubleArrowLeft from './icons/DoubleArrowLeft'; import DoubleArrowRight from './icons/DoubleArrowRight'; import { isLocationMatch } from './utils'; import { Location } from 'history'; +import { useSidebar } from './SidebarContext'; /** @public */ export type SidebarItemClassKey = @@ -369,7 +369,7 @@ const SidebarItemBase = forwardRef((props, ref) => { // XXX (@koroeskohr): unsure this is optimal. But I just really didn't want to have the item component // depend on the current location, and at least have it being optionally forced to selected. // Still waiting on a Q answered to fine tune the implementation - const { isOpen } = useContext(SidebarContext); + const { isOpen } = useSidebar(); const divStyle = !isOpen && hasSubmenu ? { display: 'flex', marginLeft: '24px' } : {}; @@ -671,7 +671,7 @@ export const SidebarScrollWrapper = styled('div')(({ theme }) => { export const SidebarExpandButton = () => { const { sidebarConfig } = useContext(SidebarConfigContext); const classes = useMemoStyles(sidebarConfig); - const { isOpen, setOpen } = useContext(SidebarContext); + const { isOpen, setOpen } = useSidebar(); const isSmallScreen = useMediaQuery( theme => theme.breakpoints.down('md'), { noSsr: true }, diff --git a/packages/core-components/src/layout/Sidebar/MobileSidebar.tsx b/packages/core-components/src/layout/Sidebar/MobileSidebar.tsx index 111bf38314..46f38d9a2c 100644 --- a/packages/core-components/src/layout/Sidebar/MobileSidebar.tsx +++ b/packages/core-components/src/layout/Sidebar/MobileSidebar.tsx @@ -27,8 +27,9 @@ import MenuIcon from '@material-ui/icons/Menu'; import { orderBy } from 'lodash'; import React, { createContext, useEffect, useState, useContext } from 'react'; import { useLocation } from 'react-router'; +import { SidebarContextProvider } from './SidebarContext'; import { SidebarGroup } from './SidebarGroup'; -import { SidebarConfigContext, SidebarContext, SidebarConfig } from './config'; +import { SidebarConfigContext, SidebarConfig } from './config'; /** * Type of `MobileSidebarContext` @@ -207,7 +208,7 @@ export const MobileSidebar = (props: MobileSidebarProps) => { !sidebarGroups[selectedMenuItemIndex].props.to; return ( - {} }}> + {} }}> @@ -231,6 +232,6 @@ export const MobileSidebar = (props: MobileSidebarProps) => { {sidebarGroups} - + ); }; diff --git a/packages/core-components/src/layout/Sidebar/SidebarContext.test.tsx b/packages/core-components/src/layout/Sidebar/SidebarContext.test.tsx new file mode 100644 index 0000000000..d14e035433 --- /dev/null +++ b/packages/core-components/src/layout/Sidebar/SidebarContext.test.tsx @@ -0,0 +1,68 @@ +/* + * Copyright 2022 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import React, { ReactNode } from 'react'; +import { renderInTestApp } from '@backstage/test-utils'; +import { screen, waitFor } from '@testing-library/react'; +import { renderHook, act } from '@testing-library/react-hooks'; +import { SidebarContextProvider, useSidebar } from './SidebarContext'; + +describe('SidebarContext', () => { + describe('SidebarContextProvider', () => { + it('should render children', async () => { + await renderInTestApp( + {} }}> + Child + , + ); + expect(await screen.findByText('Child')).toBeInTheDocument(); + }); + }); + + describe('useSidebar', () => { + it('does not need to be invoked within provider', () => { + const { result } = renderHook(() => useSidebar()); + expect(result.current.isOpen).toBe(false); + expect(typeof result.current.setOpen).toBe('function'); + }); + + it('should read and update state', async () => { + let actualValue = true; + const wrapper = ({ children }: { children: ReactNode }) => ( + { + actualValue = value; + }, + }} + > + {children} + + ); + const { result } = renderHook(() => useSidebar(), { wrapper }); + + expect(result.current.isOpen).toBe(true); + + act(() => { + result.current.setOpen(false); + }); + + waitFor(() => { + expect(result.current.isOpen).toBe(false); + }); + }); + }); +}); diff --git a/packages/core-components/src/layout/Sidebar/SidebarContext.tsx b/packages/core-components/src/layout/Sidebar/SidebarContext.tsx new file mode 100644 index 0000000000..c91be85928 --- /dev/null +++ b/packages/core-components/src/layout/Sidebar/SidebarContext.tsx @@ -0,0 +1,78 @@ +/* + * Copyright 2022 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import React, { ReactNode, useContext } from 'react'; +import { + createVersionedContext, + createVersionedValueMap, +} from '@backstage/version-bridge'; + +/** + * Types for the `SidebarContext` + * + * @public + */ +export type SidebarContextType = { + isOpen: boolean; + setOpen: (open: boolean) => void; +}; + +const VersionedSidebarContext = createVersionedContext<{ + 1: SidebarContextType; +}>('sidebar-context'); + +/** + * Provides context for reading and updating sidebar state. + * + * @public + */ +export const SidebarContextProvider = ({ + children, + value, +}: { + children: ReactNode; + value: SidebarContextType; +}) => ( + + {children} + +); + +/** + * Hook to read and update sidebar state. + * + * @public + */ +export const useSidebar = (): SidebarContextType => { + const versionedSidebarContext = useContext(VersionedSidebarContext); + + // Invoked from outside a SidbarContextProvider, return a default value. + if (versionedSidebarContext === undefined) { + return { + isOpen: false, + setOpen: () => {}, + }; + } + + const sidebarContext = versionedSidebarContext.atVersion(1); + if (sidebarContext === undefined) { + throw new Error('No context found for version 1.'); + } + + return sidebarContext; +}; diff --git a/packages/core-components/src/layout/Sidebar/SidebarSubmenu.tsx b/packages/core-components/src/layout/Sidebar/SidebarSubmenu.tsx index d1b79263eb..d8a87c98d9 100644 --- a/packages/core-components/src/layout/Sidebar/SidebarSubmenu.tsx +++ b/packages/core-components/src/layout/Sidebar/SidebarSubmenu.tsx @@ -19,10 +19,10 @@ import classnames from 'classnames'; import React, { ReactNode, useContext, useEffect, useState } from 'react'; import { SidebarItemWithSubmenuContext, - SidebarContext, SidebarConfigContext, SubmenuConfig, } from './config'; +import { useSidebar } from './SidebarContext'; import { BackstageTheme } from '@backstage/theme'; const useStyles = makeStyles< @@ -105,7 +105,7 @@ export type SidebarSubmenuProps = { * @public */ export const SidebarSubmenu = (props: SidebarSubmenuProps) => { - const { isOpen } = useContext(SidebarContext); + const { isOpen } = useSidebar(); const { sidebarConfig, submenuConfig } = useContext(SidebarConfigContext); const left = isOpen ? sidebarConfig.drawerWidthOpen diff --git a/packages/core-components/src/layout/Sidebar/config.ts b/packages/core-components/src/layout/Sidebar/config.ts index 08d469d5a3..bb0ff207fb 100644 --- a/packages/core-components/src/layout/Sidebar/config.ts +++ b/packages/core-components/src/layout/Sidebar/config.ts @@ -101,22 +101,6 @@ export const makeSidebarSubmenuConfig = ( export const SIDEBAR_INTRO_LOCAL_STORAGE = '@backstage/core/sidebar-intro-dismissed'; -/** - * Types for the `SidebarContext` - */ -export type SidebarContextType = { - isOpen: boolean; - setOpen: (open: boolean) => void; -}; - -/** - * Context whether the `Sidebar` is open - */ -export const SidebarContext = createContext({ - isOpen: false, - setOpen: () => {}, -}); - export type SidebarConfigContextType = { sidebarConfig: SidebarConfig; submenuConfig: SubmenuConfig; diff --git a/packages/core-components/src/layout/Sidebar/index.ts b/packages/core-components/src/layout/Sidebar/index.ts index ecc573fbda..a852b35bf8 100644 --- a/packages/core-components/src/layout/Sidebar/index.ts +++ b/packages/core-components/src/layout/Sidebar/index.ts @@ -54,13 +54,7 @@ export type { } from './Items'; export { IntroCard, SidebarIntro } from './Intro'; export type { SidebarIntroClassKey } from './Intro'; -export { - SIDEBAR_INTRO_LOCAL_STORAGE, - SidebarContext, - sidebarConfig, -} from './config'; -export type { - SidebarContextType, - SidebarOptions, - SubmenuOptions, -} from './config'; +export { SIDEBAR_INTRO_LOCAL_STORAGE, sidebarConfig } from './config'; +export type { SidebarOptions, SubmenuOptions } from './config'; +export { SidebarContextProvider, useSidebar } from './SidebarContext'; +export type { SidebarContextType } from './SidebarContext'; diff --git a/packages/create-app/templates/default-app/packages/app/src/components/Root/Root.tsx b/packages/create-app/templates/default-app/packages/app/src/components/Root/Root.tsx index d10eccf03a..05e11458ce 100644 --- a/packages/create-app/templates/default-app/packages/app/src/components/Root/Root.tsx +++ b/packages/create-app/templates/default-app/packages/app/src/components/Root/Root.tsx @@ -14,7 +14,7 @@ * limitations under the License. */ -import React, { useContext, PropsWithChildren } from 'react'; +import React, { PropsWithChildren } from 'react'; import { Link, makeStyles } from '@material-ui/core'; import HomeIcon from '@material-ui/icons/Home'; import ExtensionIcon from '@material-ui/icons/Extension'; @@ -32,13 +32,13 @@ import { SidebarSearchModal } from '@backstage/plugin-search'; import { Sidebar, sidebarConfig, - SidebarContext, SidebarDivider, SidebarGroup, SidebarItem, SidebarPage, SidebarScrollWrapper, SidebarSpace, + useSidebar, } from '@backstage/core-components'; import MenuIcon from '@material-ui/icons/Menu'; import SearchIcon from '@material-ui/icons/Search'; @@ -60,7 +60,7 @@ const useSidebarLogoStyles = makeStyles({ const SidebarLogo = () => { const classes = useSidebarLogoStyles(); - const { isOpen } = useContext(SidebarContext); + const { isOpen } = useSidebar(); return (
diff --git a/packages/techdocs-cli-embedded-app/src/components/Root/Root.tsx b/packages/techdocs-cli-embedded-app/src/components/Root/Root.tsx index 249394fdc4..6613e7c0ea 100644 --- a/packages/techdocs-cli-embedded-app/src/components/Root/Root.tsx +++ b/packages/techdocs-cli-embedded-app/src/components/Root/Root.tsx @@ -14,7 +14,7 @@ * limitations under the License. */ -import React, { PropsWithChildren, useContext } from 'react'; +import React, { PropsWithChildren } from 'react'; import { Link, makeStyles } from '@material-ui/core'; import LibraryBooks from '@material-ui/icons/LibraryBooks'; @@ -27,7 +27,7 @@ import { SidebarPage, sidebarConfig, SidebarDivider, - SidebarContext, + useSidebar, } from '@backstage/core-components'; import { NavLink } from 'react-router-dom'; @@ -48,7 +48,7 @@ const useSidebarLogoStyles = makeStyles({ const SidebarLogo = () => { const classes = useSidebarLogoStyles(); - const { isOpen } = useContext(SidebarContext); + const { isOpen } = useSidebar(); return (
diff --git a/plugins/shortcuts/src/ShortcutItem.test.tsx b/plugins/shortcuts/src/ShortcutItem.test.tsx index 90b8ff06e5..d1a1018513 100644 --- a/plugins/shortcuts/src/ShortcutItem.test.tsx +++ b/plugins/shortcuts/src/ShortcutItem.test.tsx @@ -20,7 +20,7 @@ import { ShortcutItem } from './ShortcutItem'; import { Shortcut } from './types'; import { LocalStoredShortcuts } from './api'; import { MockStorageApi, renderInTestApp } from '@backstage/test-utils'; -import { SidebarContext } from '@backstage/core-components'; +import { SidebarContextProvider } from '@backstage/core-components'; describe('ShortcutItem', () => { const shortcut: Shortcut = { @@ -32,9 +32,9 @@ describe('ShortcutItem', () => { it('displays the shortcut', async () => { await renderInTestApp( - {} }}> + {} }}> - , + , ); expect(screen.getByText('ST')).toBeInTheDocument(); expect(screen.getByText('some title')).toBeInTheDocument(); diff --git a/plugins/shortcuts/src/Shortcuts.test.tsx b/plugins/shortcuts/src/Shortcuts.test.tsx index 8cee9a6622..813dbb8def 100644 --- a/plugins/shortcuts/src/Shortcuts.test.tsx +++ b/plugins/shortcuts/src/Shortcuts.test.tsx @@ -24,12 +24,12 @@ import { screen, waitFor } from '@testing-library/react'; import { Shortcuts } from './Shortcuts'; import { LocalStoredShortcuts, shortcutsApiRef } from './api'; -import { SidebarContext } from '@backstage/core-components'; +import { SidebarContextProvider } from '@backstage/core-components'; describe('Shortcuts', () => { it('displays an add button', async () => { await renderInTestApp( - {} }}> + {} }}> { > - , + , ); await waitFor(() => !screen.queryByTestId('progress')); expect(screen.getByText('Add Shortcuts')).toBeInTheDocument(); From da72da5daee9ac5cd54e43bbc9c2db13b51b6f53 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Fri, 20 May 2022 15:23:39 +0200 Subject: [PATCH 2/7] Replace SidebarPinStateContext with versioned provider and hook. Signed-off-by: Eric Peterson --- .../app/src/components/search/SearchPage.tsx | 6 +- packages/core-components/api-report.md | 11 ++- .../core-components/src/layout/Page/Page.tsx | 6 +- .../src/layout/Sidebar/Bar.test.tsx | 6 +- .../src/layout/Sidebar/Bar.tsx | 9 +-- .../src/layout/Sidebar/Page.tsx | 29 +------ .../src/layout/Sidebar/SidebarGroup.tsx | 4 +- .../Sidebar/SidebarPinStateContext.test.tsx | 79 +++++++++++++++++++ .../layout/Sidebar/SidebarPinStateContext.tsx | 79 +++++++++++++++++++ .../src/layout/Sidebar/index.ts | 17 ++-- .../reader/transformers/styles/transformer.ts | 6 +- .../General/UserSettingsAppearanceCard.tsx | 6 +- .../General/UserSettingsPinToggle.test.tsx | 6 +- .../General/UserSettingsPinToggle.tsx | 8 +- .../src/components/SettingsPage.tsx | 6 +- 15 files changed, 208 insertions(+), 70 deletions(-) create mode 100644 packages/core-components/src/layout/Sidebar/SidebarPinStateContext.test.tsx create mode 100644 packages/core-components/src/layout/Sidebar/SidebarPinStateContext.tsx diff --git a/packages/app/src/components/search/SearchPage.tsx b/packages/app/src/components/search/SearchPage.tsx index d1b56bbaf0..d23ad957d7 100644 --- a/packages/app/src/components/search/SearchPage.tsx +++ b/packages/app/src/components/search/SearchPage.tsx @@ -21,7 +21,7 @@ import { Header, Lifecycle, Page, - SidebarPinStateContext, + useSidebarPinState, } from '@backstage/core-components'; import { useApi } from '@backstage/core-plugin-api'; import { CatalogSearchResultListItem } from '@backstage/plugin-catalog'; @@ -40,7 +40,7 @@ import { import { useSearch } from '@backstage/plugin-search-react'; import { TechDocsSearchResultListItem } from '@backstage/plugin-techdocs'; import { Grid, List, makeStyles, Paper, Theme } from '@material-ui/core'; -import React, { useContext } from 'react'; +import React from 'react'; const useStyles = makeStyles((theme: Theme) => ({ bar: { @@ -59,7 +59,7 @@ const useStyles = makeStyles((theme: Theme) => ({ const SearchPage = () => { const classes = useStyles(); - const { isMobile } = useContext(SidebarPinStateContext); + const { isMobile } = useSidebarPinState(); const { types } = useSearch(); const catalogApi = useApi(catalogApiRef); diff --git a/packages/core-components/api-report.md b/packages/core-components/api-report.md index e7ccc8e72b..d162e7a714 100644 --- a/packages/core-components/api-report.md +++ b/packages/core-components/api-report.md @@ -1006,7 +1006,13 @@ export type SidebarPageProps = { }; // @public -export const SidebarPinStateContext: React_2.Context; +export const SidebarPinStateContextProvider: ({ + children, + value, +}: { + children: ReactNode; + value: SidebarPinStateContextType; +}) => JSX.Element; // @public export type SidebarPinStateContextType = { @@ -1451,6 +1457,9 @@ export class UserIdentity implements IdentityApi { // @public export const useSidebar: () => SidebarContextType; +// @public +export const useSidebarPinState: () => SidebarPinStateContextType; + // Warning: (ae-missing-release-tag) "useSupportConfig" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) // // @public (undocumented) diff --git a/packages/core-components/src/layout/Page/Page.tsx b/packages/core-components/src/layout/Page/Page.tsx index 9bfc16fea3..1abb8da93b 100644 --- a/packages/core-components/src/layout/Page/Page.tsx +++ b/packages/core-components/src/layout/Page/Page.tsx @@ -14,10 +14,10 @@ * limitations under the License. */ -import React, { useContext } from 'react'; +import React from 'react'; import { BackstageTheme } from '@backstage/theme'; import { makeStyles, ThemeProvider } from '@material-ui/core/styles'; -import { SidebarPinStateContext } from '../Sidebar/Page'; +import { useSidebarPinState } from '../Sidebar/SidebarPinStateContext'; export type PageClassKey = 'root'; @@ -43,7 +43,7 @@ type Props = { export function Page(props: Props) { const { themeId, children } = props; - const { isMobile } = useContext(SidebarPinStateContext); + const { isMobile } = useSidebarPinState(); const classes = useStyles({ isMobile }); return ( - , + , ); } diff --git a/packages/core-components/src/layout/Sidebar/Bar.tsx b/packages/core-components/src/layout/Sidebar/Bar.tsx index 2fdb2012dc..a315406b58 100644 --- a/packages/core-components/src/layout/Sidebar/Bar.tsx +++ b/packages/core-components/src/layout/Sidebar/Bar.tsx @@ -31,8 +31,9 @@ import { SubmenuOptions, } from './config'; import { BackstageTheme } from '@backstage/theme'; -import { SidebarPinStateContext, useContent } from './Page'; +import { useContent } from './Page'; import { SidebarContextProvider } from './SidebarContext'; +import { useSidebarPinState } from './SidebarPinStateContext'; import { MobileSidebar } from './MobileSidebar'; /** @public */ @@ -133,9 +134,7 @@ const DesktopSidebar = (props: DesktopSidebarProps) => { ); const [state, setState] = useState(State.Closed); const hoverTimerRef = useRef(); - const { isPinned, toggleSidebarPinState } = useContext( - SidebarPinStateContext, - ); + const { isPinned, toggleSidebarPinState } = useSidebarPinState(); const handleOpen = () => { if (isPinned || disableExpandOnHover) { @@ -226,7 +225,7 @@ export const Sidebar = (props: SidebarProps) => { props.submenuOptions ?? {}, ); const { children, disableExpandOnHover, openDelayMs, closeDelayMs } = props; - const { isMobile } = useContext(SidebarPinStateContext); + const { isMobile } = useSidebarPinState(); return isMobile ? ( {children} diff --git a/packages/core-components/src/layout/Sidebar/Page.tsx b/packages/core-components/src/layout/Sidebar/Page.tsx index 9bf8105f81..17cd5ddee1 100644 --- a/packages/core-components/src/layout/Sidebar/Page.tsx +++ b/packages/core-components/src/layout/Sidebar/Page.tsx @@ -29,6 +29,7 @@ import { SidebarConfigContext, SidebarConfig } from './config'; import { BackstageTheme } from '@backstage/theme'; import { LocalStorage } from './localStorage'; import useMediaQuery from '@material-ui/core/useMediaQuery'; +import { SidebarPinStateContextProvider } from './SidebarPinStateContext'; export type SidebarPageClassKey = 'root'; @@ -62,17 +63,6 @@ const useStyles = makeStyles< { name: 'BackstageSidebarPage' }, ); -/** - * Type of `SidebarPinStateContext` - * - * @public - */ -export type SidebarPinStateContextType = { - isPinned: boolean; - toggleSidebarPinState: () => any; - isMobile?: boolean; -}; - /** * Props for SidebarPage * @@ -82,19 +72,6 @@ export type SidebarPageProps = { children?: React.ReactNode; }; -/** - * Contains the state on how the `Sidebar` is rendered - * - * @public - */ -export const SidebarPinStateContext = createContext( - { - isPinned: true, - toggleSidebarPinState: () => {}, - isMobile: false, - }, -); - type PageContextType = { content: { contentRef?: React.MutableRefObject; @@ -137,7 +114,7 @@ export function SidebarPage(props: SidebarPageProps) { const classes = useStyles({ isPinned, sidebarConfig }); return ( -
{props.children}
- + ); } diff --git a/packages/core-components/src/layout/Sidebar/SidebarGroup.tsx b/packages/core-components/src/layout/Sidebar/SidebarGroup.tsx index 55b5e770b0..1158cc14df 100644 --- a/packages/core-components/src/layout/Sidebar/SidebarGroup.tsx +++ b/packages/core-components/src/layout/Sidebar/SidebarGroup.tsx @@ -22,7 +22,7 @@ import BottomNavigationAction, { import { makeStyles } from '@material-ui/core/styles'; import React, { useContext } from 'react'; import { useLocation } from 'react-router-dom'; -import { SidebarPinStateContext } from '.'; +import { useSidebarPinState } from '.'; import { Link } from '../../components'; import { SidebarConfigContext, SidebarConfig } from './config'; import { MobileSidebarContext } from './MobileSidebar'; @@ -122,7 +122,7 @@ const MobileSidebarGroup = (props: SidebarGroupProps) => { */ export const SidebarGroup = (props: SidebarGroupProps) => { const { children, to, label, icon, value } = props; - const { isMobile } = useContext(SidebarPinStateContext); + const { isMobile } = useSidebarPinState(); return isMobile ? ( diff --git a/packages/core-components/src/layout/Sidebar/SidebarPinStateContext.test.tsx b/packages/core-components/src/layout/Sidebar/SidebarPinStateContext.test.tsx new file mode 100644 index 0000000000..cc83a846e3 --- /dev/null +++ b/packages/core-components/src/layout/Sidebar/SidebarPinStateContext.test.tsx @@ -0,0 +1,79 @@ +/* + * Copyright 2022 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import React, { ReactNode } from 'react'; +import { renderInTestApp } from '@backstage/test-utils'; +import { screen, waitFor } from '@testing-library/react'; +import { renderHook, act } from '@testing-library/react-hooks'; +import { + SidebarPinStateContextProvider, + useSidebarPinState, +} from './SidebarPinStateContext'; + +describe('SidebarContext', () => { + describe('SidebarContextProvider', () => { + it('should render children', async () => { + await renderInTestApp( + {}, + }} + > + Child + , + ); + expect(await screen.findByText('Child')).toBeInTheDocument(); + }); + }); + + describe('useSidebar', () => { + it('does not need to be invoked within provider', () => { + const { result } = renderHook(() => useSidebarPinState()); + expect(result.current.isPinned).toBe(true); + expect(result.current.isMobile).toBe(false); + expect(typeof result.current.toggleSidebarPinState).toBe('function'); + }); + + it('should read and update state', async () => { + let actualValue = true; + const wrapper = ({ children }: { children: ReactNode }) => ( + { + actualValue = !actualValue; + }, + }} + > + {children} + + ); + const { result } = renderHook(() => useSidebarPinState(), { wrapper }); + + expect(result.current.isPinned).toBe(true); + + act(() => { + result.current.toggleSidebarPinState(); + }); + + waitFor(() => { + expect(result.current.isPinned).toBe(false); + }); + }); + }); +}); diff --git a/packages/core-components/src/layout/Sidebar/SidebarPinStateContext.tsx b/packages/core-components/src/layout/Sidebar/SidebarPinStateContext.tsx new file mode 100644 index 0000000000..709973e064 --- /dev/null +++ b/packages/core-components/src/layout/Sidebar/SidebarPinStateContext.tsx @@ -0,0 +1,79 @@ +/* + * Copyright 2022 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import { + createVersionedContext, + createVersionedValueMap, +} from '@backstage/version-bridge'; +import React, { ReactNode, useContext } from 'react'; + +/** + * Type of `SidebarPinStateContext` + * + * @public + */ +export type SidebarPinStateContextType = { + isPinned: boolean; + toggleSidebarPinState: () => any; + isMobile?: boolean; +}; + +const VersionedSidebarPinStateContext = createVersionedContext<{ + 1: SidebarPinStateContextType; +}>('sidebar-pin-state-context'); + +/** + * Provides state for how the `Sidebar` is rendered + * + * @public + */ +export const SidebarPinStateContextProvider = ({ + children, + value, +}: { + children: ReactNode; + value: SidebarPinStateContextType; +}) => ( + + {children} + +); + +/** + * Hook to read and update sidebar pin state. + * + * @public + */ +export const useSidebarPinState = (): SidebarPinStateContextType => { + const versionedSidebarContext = useContext(VersionedSidebarPinStateContext); + + // Invoked from outside a SidebarPinStateContextProvider: default value. + if (versionedSidebarContext === undefined) { + return { + isPinned: true, + toggleSidebarPinState: () => {}, + isMobile: false, + }; + } + + const sidebarContext = versionedSidebarContext.atVersion(1); + if (sidebarContext === undefined) { + throw new Error('No context found for version 1.'); + } + + return sidebarContext; +}; diff --git a/packages/core-components/src/layout/Sidebar/index.ts b/packages/core-components/src/layout/Sidebar/index.ts index a852b35bf8..e041ecc730 100644 --- a/packages/core-components/src/layout/Sidebar/index.ts +++ b/packages/core-components/src/layout/Sidebar/index.ts @@ -27,16 +27,8 @@ export type { SidebarSubmenuItemDropdownItem, } from './SidebarSubmenuItem'; export type { SidebarClassKey, SidebarProps } from './Bar'; -export { - SidebarPage, - SidebarPinStateContext as SidebarPinStateContext, - useContent, -} from './Page'; -export type { - SidebarPinStateContextType as SidebarPinStateContextType, - SidebarPageClassKey, - SidebarPageProps, -} from './Page'; +export { SidebarPage, useContent } from './Page'; +export type { SidebarPageClassKey, SidebarPageProps } from './Page'; export { SidebarDivider, SidebarItem, @@ -58,3 +50,8 @@ export { SIDEBAR_INTRO_LOCAL_STORAGE, sidebarConfig } from './config'; export type { SidebarOptions, SubmenuOptions } from './config'; export { SidebarContextProvider, useSidebar } from './SidebarContext'; export type { SidebarContextType } from './SidebarContext'; +export { + SidebarPinStateContextProvider, + useSidebarPinState, +} from './SidebarPinStateContext'; +export type { SidebarPinStateContextType } from './SidebarPinStateContext'; diff --git a/plugins/techdocs/src/reader/transformers/styles/transformer.ts b/plugins/techdocs/src/reader/transformers/styles/transformer.ts index 26ab4ceb26..04a6d476af 100644 --- a/plugins/techdocs/src/reader/transformers/styles/transformer.ts +++ b/plugins/techdocs/src/reader/transformers/styles/transformer.ts @@ -14,11 +14,11 @@ * limitations under the License. */ -import { useCallback, useContext, useMemo } from 'react'; +import { useCallback, useMemo } from 'react'; import { useTheme } from '@material-ui/core'; -import { SidebarPinStateContext } from '@backstage/core-components'; +import { useSidebarPinState } from '@backstage/core-components'; import { BackstageTheme } from '@backstage/theme'; import { Transformer } from '../transformer'; @@ -27,7 +27,7 @@ import { rules } from './rules'; /** * Sidebar pinned state to be used in computing style injections. */ -const useSidebar = () => useContext(SidebarPinStateContext); +const useSidebar = () => useSidebarPinState(); /** * Process all rules and concatenate their definitions into a single style. diff --git a/plugins/user-settings/src/components/General/UserSettingsAppearanceCard.tsx b/plugins/user-settings/src/components/General/UserSettingsAppearanceCard.tsx index 15ab93c6d0..cd55218db5 100644 --- a/plugins/user-settings/src/components/General/UserSettingsAppearanceCard.tsx +++ b/plugins/user-settings/src/components/General/UserSettingsAppearanceCard.tsx @@ -13,14 +13,14 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { InfoCard, SidebarPinStateContext } from '@backstage/core-components'; +import { InfoCard, useSidebarPinState } from '@backstage/core-components'; import { List } from '@material-ui/core'; -import React, { useContext } from 'react'; +import React from 'react'; import { UserSettingsPinToggle } from './UserSettingsPinToggle'; import { UserSettingsThemeToggle } from './UserSettingsThemeToggle'; export const UserSettingsAppearanceCard = () => { - const { isMobile } = useContext(SidebarPinStateContext); + const { isMobile } = useSidebarPinState(); return ( diff --git a/plugins/user-settings/src/components/General/UserSettingsPinToggle.test.tsx b/plugins/user-settings/src/components/General/UserSettingsPinToggle.test.tsx index b63ac85b10..a41792768a 100644 --- a/plugins/user-settings/src/components/General/UserSettingsPinToggle.test.tsx +++ b/plugins/user-settings/src/components/General/UserSettingsPinToggle.test.tsx @@ -18,14 +18,14 @@ import { renderWithEffects, wrapInTestApp } from '@backstage/test-utils'; import { fireEvent } from '@testing-library/react'; import React from 'react'; import { UserSettingsPinToggle } from './UserSettingsPinToggle'; -import { SidebarPinStateContext } from '@backstage/core-components'; +import { SidebarPinStateContextProvider } from '@backstage/core-components'; describe('', () => { it('toggles the pin sidebar button', async () => { const mockToggleFn = jest.fn(); const rendered = await renderWithEffects( wrapInTestApp( - ', () => { }} > - , + , ), ); expect(rendered.getByText('Pin Sidebar')).toBeInTheDocument(); diff --git a/plugins/user-settings/src/components/General/UserSettingsPinToggle.tsx b/plugins/user-settings/src/components/General/UserSettingsPinToggle.tsx index 4d71df8113..d218787c05 100644 --- a/plugins/user-settings/src/components/General/UserSettingsPinToggle.tsx +++ b/plugins/user-settings/src/components/General/UserSettingsPinToggle.tsx @@ -14,7 +14,7 @@ * limitations under the License. */ -import React, { useContext } from 'react'; +import React from 'react'; import { ListItem, ListItemSecondaryAction, @@ -22,12 +22,10 @@ import { Switch, Tooltip, } from '@material-ui/core'; -import { SidebarPinStateContext } from '@backstage/core-components'; +import { useSidebarPinState } from '@backstage/core-components'; export const UserSettingsPinToggle = () => { - const { isPinned, toggleSidebarPinState } = useContext( - SidebarPinStateContext, - ); + const { isPinned, toggleSidebarPinState } = useSidebarPinState(); return ( diff --git a/plugins/user-settings/src/components/SettingsPage.tsx b/plugins/user-settings/src/components/SettingsPage.tsx index 8942235fda..dcaf62d435 100644 --- a/plugins/user-settings/src/components/SettingsPage.tsx +++ b/plugins/user-settings/src/components/SettingsPage.tsx @@ -17,10 +17,10 @@ import { Header, Page, - SidebarPinStateContext, TabbedLayout, + useSidebarPinState, } from '@backstage/core-components'; -import React, { useContext } from 'react'; +import React from 'react'; import { useOutlet } from 'react-router'; import { useElementFilter } from '@backstage/core-plugin-api'; import { UserSettingsAuthProviders } from './AuthProviders'; @@ -33,7 +33,7 @@ type Props = { }; export const SettingsPage = ({ providerSettings }: Props) => { - const { isMobile } = useContext(SidebarPinStateContext); + const { isMobile } = useSidebarPinState(); const outlet = useOutlet(); const tabs = useElementFilter(outlet, elements => From bff65e6958de04600bfcfe4bcd022cc6e0443b8f Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Fri, 20 May 2022 16:51:32 +0200 Subject: [PATCH 3/7] Changesets for affected packages. Signed-off-by: Eric Peterson --- .changeset/give-that-wolf-a-banana.md | 30 +++++++++++++++++++++++ .changeset/right-one-at-the-wrong-time.md | 7 ++++++ .changeset/up-in-space-man.md | 7 ++++++ 3 files changed, 44 insertions(+) create mode 100644 .changeset/give-that-wolf-a-banana.md create mode 100644 .changeset/right-one-at-the-wrong-time.md create mode 100644 .changeset/up-in-space-man.md diff --git a/.changeset/give-that-wolf-a-banana.md b/.changeset/give-that-wolf-a-banana.md new file mode 100644 index 0000000000..685ef7400a --- /dev/null +++ b/.changeset/give-that-wolf-a-banana.md @@ -0,0 +1,30 @@ +--- +'@backstage/create-app': patch +--- + +Use of `SidebarContext` has been deprecated and will be removed in a future release. Instead, `useSidebar()` should be used to consume the context and `` should be used to provide it. + +To prepare your app, update `packages/app/src/components/Root/Root.tsx` as follows: + +```diff +import { + Sidebar, + sidebarConfig, +- SidebarContext + SidebarDivider, + // ... + SidebarSpace, ++ useSidebar, +} from '@backstage/core-components'; + +// ... + + +const SidebarLogo = () => { + const classes = useSidebarLogoStyles(); +- const { isOpen } = useContext(SidebarContext); ++ const { isOpen } = useSidebar(); + + // ... +}; +``` diff --git a/.changeset/right-one-at-the-wrong-time.md b/.changeset/right-one-at-the-wrong-time.md new file mode 100644 index 0000000000..c7fee94a91 --- /dev/null +++ b/.changeset/right-one-at-the-wrong-time.md @@ -0,0 +1,7 @@ +--- +'@backstage/plugin-techdocs': patch +'@backstage/plugin-user-settings': patch +'@techdocs/cli': patch +--- + +Updated sidebar-related logic to use `` + `useSidebarPinState()` and/or `` + `useSidebar()` from `@backstage/core-components`. diff --git a/.changeset/up-in-space-man.md b/.changeset/up-in-space-man.md new file mode 100644 index 0000000000..f3aadfece8 --- /dev/null +++ b/.changeset/up-in-space-man.md @@ -0,0 +1,7 @@ +--- +'@backstage/core-components': patch +--- + +The `SidebarPinStateContext` and `SidebarContext` have been deprecated and will be removed in a future release. Instead, use `` + `useSidebarPinState()` and/or `` + `useSidebar()`. + +This was done to ensure that sidebar state can be shared successfully across components exported by different packages, regardless of what version of this package is resolved and installed for each individual package. From a907d620fa771fc4a4c8f71e4adabdb2839a3446 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Fri, 20 May 2022 17:28:14 +0200 Subject: [PATCH 4/7] Provide a deprecation path for affected sidebar contexts. Signed-off-by: Eric Peterson --- packages/core-components/api-report.md | 6 +++ .../layout/Sidebar/SidebarContext.test.tsx | 41 +++++++++++++---- .../src/layout/Sidebar/SidebarContext.tsx | 34 +++++++++----- .../Sidebar/SidebarPinStateContext.test.tsx | 45 ++++++++++++++----- .../layout/Sidebar/SidebarPinStateContext.tsx | 35 ++++++++++----- .../src/layout/Sidebar/index.ts | 7 ++- 6 files changed, 128 insertions(+), 40 deletions(-) diff --git a/packages/core-components/api-report.md b/packages/core-components/api-report.md index d162e7a714..33db173fcf 100644 --- a/packages/core-components/api-report.md +++ b/packages/core-components/api-report.md @@ -902,6 +902,9 @@ export const sidebarConfig: { mobileSidebarHeight: number; }; +// @public @deprecated +export const SidebarContext: React_2.Context; + // @public export const SidebarContextProvider: ({ children, @@ -1005,6 +1008,9 @@ export type SidebarPageProps = { children?: React_2.ReactNode; }; +// @public @deprecated +export const SidebarPinStateContext: React_2.Context; + // @public export const SidebarPinStateContextProvider: ({ children, diff --git a/packages/core-components/src/layout/Sidebar/SidebarContext.test.tsx b/packages/core-components/src/layout/Sidebar/SidebarContext.test.tsx index d14e035433..43a18851d7 100644 --- a/packages/core-components/src/layout/Sidebar/SidebarContext.test.tsx +++ b/packages/core-components/src/layout/Sidebar/SidebarContext.test.tsx @@ -13,21 +13,45 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import React, { ReactNode } from 'react'; -import { renderInTestApp } from '@backstage/test-utils'; -import { screen, waitFor } from '@testing-library/react'; +import React, { ReactNode, useContext } from 'react'; +import { renderWithEffects } from '@backstage/test-utils'; +import { waitFor } from '@testing-library/react'; import { renderHook, act } from '@testing-library/react-hooks'; -import { SidebarContextProvider, useSidebar } from './SidebarContext'; +import { + LegacySidebarContext, + SidebarContextProvider, + useSidebar, +} from './SidebarContext'; describe('SidebarContext', () => { describe('SidebarContextProvider', () => { it('should render children', async () => { - await renderInTestApp( + const { findByText } = await renderWithEffects( {} }}> Child , ); - expect(await screen.findByText('Child')).toBeInTheDocument(); + expect(await findByText('Child')).toBeInTheDocument(); + }); + + it('should provide the legacy context as well, for now', async () => { + const LegacyContextSpy = () => { + const { isOpen } = useContext(LegacySidebarContext); + return <>{String(isOpen)}; + }; + + const { findByText } = await renderWithEffects( + {}, + }} + > + + , + ); + + expect(await findByText('true')).toBeInTheDocument(); }); }); @@ -52,15 +76,16 @@ describe('SidebarContext', () => { {children} ); - const { result } = renderHook(() => useSidebar(), { wrapper }); + const { result, rerender } = renderHook(() => useSidebar(), { wrapper }); expect(result.current.isOpen).toBe(true); act(() => { result.current.setOpen(false); + rerender(); }); - waitFor(() => { + await waitFor(() => { expect(result.current.isOpen).toBe(false); }); }); diff --git a/packages/core-components/src/layout/Sidebar/SidebarContext.tsx b/packages/core-components/src/layout/Sidebar/SidebarContext.tsx index c91be85928..4e47b6040a 100644 --- a/packages/core-components/src/layout/Sidebar/SidebarContext.tsx +++ b/packages/core-components/src/layout/Sidebar/SidebarContext.tsx @@ -14,7 +14,7 @@ * limitations under the License. */ -import React, { ReactNode, useContext } from 'react'; +import React, { createContext, ReactNode, useContext } from 'react'; import { createVersionedContext, createVersionedValueMap, @@ -30,6 +30,21 @@ export type SidebarContextType = { setOpen: (open: boolean) => void; }; +const defaultSidebarContext = { + isOpen: false, + setOpen: () => {}, +}; + +/** + * Context whether the `Sidebar` is open + * + * @public @deprecated + * Use `` + `useSidebar()` instead. + */ +export const LegacySidebarContext = createContext( + defaultSidebarContext, +); + const VersionedSidebarContext = createVersionedContext<{ 1: SidebarContextType; }>('sidebar-context'); @@ -46,11 +61,13 @@ export const SidebarContextProvider = ({ children: ReactNode; value: SidebarContextType; }) => ( - - {children} - + + + {children} + + ); /** @@ -63,10 +80,7 @@ export const useSidebar = (): SidebarContextType => { // Invoked from outside a SidbarContextProvider, return a default value. if (versionedSidebarContext === undefined) { - return { - isOpen: false, - setOpen: () => {}, - }; + return defaultSidebarContext; } const sidebarContext = versionedSidebarContext.atVersion(1); diff --git a/packages/core-components/src/layout/Sidebar/SidebarPinStateContext.test.tsx b/packages/core-components/src/layout/Sidebar/SidebarPinStateContext.test.tsx index cc83a846e3..d8bc054e4a 100644 --- a/packages/core-components/src/layout/Sidebar/SidebarPinStateContext.test.tsx +++ b/packages/core-components/src/layout/Sidebar/SidebarPinStateContext.test.tsx @@ -13,19 +13,20 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import React, { ReactNode } from 'react'; -import { renderInTestApp } from '@backstage/test-utils'; -import { screen, waitFor } from '@testing-library/react'; +import React, { ReactNode, useContext } from 'react'; +import { renderWithEffects } from '@backstage/test-utils'; +import { waitFor } from '@testing-library/react'; import { renderHook, act } from '@testing-library/react-hooks'; import { + LegacySidebarPinStateContext, SidebarPinStateContextProvider, useSidebarPinState, } from './SidebarPinStateContext'; -describe('SidebarContext', () => { - describe('SidebarContextProvider', () => { +describe('SidebarPinStateContext', () => { + describe('SidebarPinStateContextProvider', () => { it('should render children', async () => { - await renderInTestApp( + const { findByText } = await renderWithEffects( { Child , ); - expect(await screen.findByText('Child')).toBeInTheDocument(); + expect(await findByText('Child')).toBeInTheDocument(); + }); + + it('should provide the legacy context as well, for now', async () => { + const LegacyContextSpy = () => { + const { isMobile } = useContext(LegacySidebarPinStateContext); + return <>{String(isMobile)}; + }; + + const { findByText } = await renderWithEffects( + {}, + }} + > + + , + ); + + expect(await findByText('true')).toBeInTheDocument(); }); }); - describe('useSidebar', () => { + describe('useSidebarPinState', () => { it('does not need to be invoked within provider', () => { const { result } = renderHook(() => useSidebarPinState()); expect(result.current.isPinned).toBe(true); @@ -63,15 +85,18 @@ describe('SidebarContext', () => { {children} ); - const { result } = renderHook(() => useSidebarPinState(), { wrapper }); + const { result, rerender } = renderHook(() => useSidebarPinState(), { + wrapper, + }); expect(result.current.isPinned).toBe(true); act(() => { result.current.toggleSidebarPinState(); + rerender(); }); - waitFor(() => { + await waitFor(() => { expect(result.current.isPinned).toBe(false); }); }); diff --git a/packages/core-components/src/layout/Sidebar/SidebarPinStateContext.tsx b/packages/core-components/src/layout/Sidebar/SidebarPinStateContext.tsx index 709973e064..ec38884890 100644 --- a/packages/core-components/src/layout/Sidebar/SidebarPinStateContext.tsx +++ b/packages/core-components/src/layout/Sidebar/SidebarPinStateContext.tsx @@ -17,7 +17,7 @@ import { createVersionedContext, createVersionedValueMap, } from '@backstage/version-bridge'; -import React, { ReactNode, useContext } from 'react'; +import React, { createContext, ReactNode, useContext } from 'react'; /** * Type of `SidebarPinStateContext` @@ -30,6 +30,21 @@ export type SidebarPinStateContextType = { isMobile?: boolean; }; +const defaultSidebarPinStateContext = { + isPinned: true, + toggleSidebarPinState: () => {}, + isMobile: false, +}; + +/** + * Contains the state on how the `Sidebar` is rendered + * + * @public @deprecated + * Use `` + `useSidebarPinState()` instead. + */ +export const LegacySidebarPinStateContext = + createContext(defaultSidebarPinStateContext); + const VersionedSidebarPinStateContext = createVersionedContext<{ 1: SidebarPinStateContextType; }>('sidebar-pin-state-context'); @@ -46,11 +61,13 @@ export const SidebarPinStateContextProvider = ({ children: ReactNode; value: SidebarPinStateContextType; }) => ( - - {children} - + + + {children} + + ); /** @@ -63,11 +80,7 @@ export const useSidebarPinState = (): SidebarPinStateContextType => { // Invoked from outside a SidebarPinStateContextProvider: default value. if (versionedSidebarContext === undefined) { - return { - isPinned: true, - toggleSidebarPinState: () => {}, - isMobile: false, - }; + return defaultSidebarPinStateContext; } const sidebarContext = versionedSidebarContext.atVersion(1); diff --git a/packages/core-components/src/layout/Sidebar/index.ts b/packages/core-components/src/layout/Sidebar/index.ts index e041ecc730..06681ded52 100644 --- a/packages/core-components/src/layout/Sidebar/index.ts +++ b/packages/core-components/src/layout/Sidebar/index.ts @@ -48,9 +48,14 @@ export { IntroCard, SidebarIntro } from './Intro'; export type { SidebarIntroClassKey } from './Intro'; export { SIDEBAR_INTRO_LOCAL_STORAGE, sidebarConfig } from './config'; export type { SidebarOptions, SubmenuOptions } from './config'; -export { SidebarContextProvider, useSidebar } from './SidebarContext'; +export { + LegacySidebarContext as SidebarContext, + SidebarContextProvider, + useSidebar, +} from './SidebarContext'; export type { SidebarContextType } from './SidebarContext'; export { + LegacySidebarPinStateContext as SidebarPinStateContext, SidebarPinStateContextProvider, useSidebarPinState, } from './SidebarPinStateContext'; From 37c8f8444c46b5ca442bd09f7f8685d4932244a3 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Fri, 20 May 2022 17:51:11 +0200 Subject: [PATCH 5/7] Better naming of providers, hooks, and types. Signed-off-by: Eric Peterson --- .changeset/give-that-wolf-a-banana.md | 7 ++- .changeset/right-one-at-the-wrong-time.md | 2 +- .changeset/up-in-space-man.md | 2 +- packages/app/src/components/Root/Root.tsx | 4 +- packages/core-components/api-report.md | 53 ++++++++++++------- .../src/layout/Sidebar/Bar.test.tsx | 6 +-- .../src/layout/Sidebar/Bar.tsx | 6 +-- .../src/layout/Sidebar/Intro.tsx | 4 +- .../src/layout/Sidebar/Items.tsx | 6 +-- .../src/layout/Sidebar/MobileSidebar.tsx | 6 +-- .../src/layout/Sidebar/Page.tsx | 6 +-- ...t.tsx => SidebarOpenStateContext.test.tsx} | 30 ++++++----- ...ontext.tsx => SidebarOpenStateContext.tsx} | 27 +++++++--- .../Sidebar/SidebarPinStateContext.test.tsx | 16 +++--- .../layout/Sidebar/SidebarPinStateContext.tsx | 22 ++++++-- .../src/layout/Sidebar/SidebarSubmenu.tsx | 4 +- .../src/layout/Sidebar/index.ts | 18 ++++--- .../packages/app/src/components/Root/Root.tsx | 4 +- .../src/components/Root/Root.tsx | 4 +- plugins/shortcuts/src/ShortcutItem.test.tsx | 6 +-- plugins/shortcuts/src/Shortcuts.test.tsx | 6 +-- .../General/UserSettingsPinToggle.test.tsx | 6 +-- 22 files changed, 144 insertions(+), 101 deletions(-) rename packages/core-components/src/layout/Sidebar/{SidebarContext.test.tsx => SidebarOpenStateContext.test.tsx} (78%) rename packages/core-components/src/layout/Sidebar/{SidebarContext.tsx => SidebarOpenStateContext.tsx} (79%) diff --git a/.changeset/give-that-wolf-a-banana.md b/.changeset/give-that-wolf-a-banana.md index 685ef7400a..a9e99782ad 100644 --- a/.changeset/give-that-wolf-a-banana.md +++ b/.changeset/give-that-wolf-a-banana.md @@ -2,7 +2,7 @@ '@backstage/create-app': patch --- -Use of `SidebarContext` has been deprecated and will be removed in a future release. Instead, `useSidebar()` should be used to consume the context and `` should be used to provide it. +Use of `SidebarContext` has been deprecated and will be removed in a future release. Instead, `useSidebarOpenState()` should be used to consume the context and `` should be used to provide it. To prepare your app, update `packages/app/src/components/Root/Root.tsx` as follows: @@ -14,16 +14,15 @@ import { SidebarDivider, // ... SidebarSpace, -+ useSidebar, ++ useSidebarOpenState, } from '@backstage/core-components'; // ... - const SidebarLogo = () => { const classes = useSidebarLogoStyles(); - const { isOpen } = useContext(SidebarContext); -+ const { isOpen } = useSidebar(); ++ const { isOpen } = useSidebarOpenState(); // ... }; diff --git a/.changeset/right-one-at-the-wrong-time.md b/.changeset/right-one-at-the-wrong-time.md index c7fee94a91..5ef8e42555 100644 --- a/.changeset/right-one-at-the-wrong-time.md +++ b/.changeset/right-one-at-the-wrong-time.md @@ -4,4 +4,4 @@ '@techdocs/cli': patch --- -Updated sidebar-related logic to use `` + `useSidebarPinState()` and/or `` + `useSidebar()` from `@backstage/core-components`. +Updated sidebar-related logic to use `` + `useSidebarPinState()` and/or `` + `useSidebarOpenState()` from `@backstage/core-components`. diff --git a/.changeset/up-in-space-man.md b/.changeset/up-in-space-man.md index f3aadfece8..3a4dc6c5d3 100644 --- a/.changeset/up-in-space-man.md +++ b/.changeset/up-in-space-man.md @@ -2,6 +2,6 @@ '@backstage/core-components': patch --- -The `SidebarPinStateContext` and `SidebarContext` have been deprecated and will be removed in a future release. Instead, use `` + `useSidebarPinState()` and/or `` + `useSidebar()`. +The `SidebarPinStateContext` and `SidebarContext` have been deprecated and will be removed in a future release. Instead, use `` + `useSidebarPinState()` and/or `` + `useSidebarOpenState()`. This was done to ensure that sidebar state can be shared successfully across components exported by different packages, regardless of what version of this package is resolved and installed for each individual package. diff --git a/packages/app/src/components/Root/Root.tsx b/packages/app/src/components/Root/Root.tsx index e90fe29b54..30da01841c 100644 --- a/packages/app/src/components/Root/Root.tsx +++ b/packages/app/src/components/Root/Root.tsx @@ -45,7 +45,7 @@ import { SidebarPage, SidebarScrollWrapper, SidebarSpace, - useSidebar, + useSidebarOpenState, } from '@backstage/core-components'; import { MyGroupsSidebarItem } from '@backstage/plugin-org'; import GroupIcon from '@material-ui/icons/People'; @@ -68,7 +68,7 @@ const useSidebarLogoStyles = makeStyles({ const SidebarLogo = () => { const classes = useSidebarLogoStyles(); - const { isOpen } = useSidebar(); + const { isOpen } = useSidebarOpenState(); return (
diff --git a/packages/core-components/api-report.md b/packages/core-components/api-report.md index 33db173fcf..911ef36867 100644 --- a/packages/core-components/api-report.md +++ b/packages/core-components/api-report.md @@ -905,16 +905,7 @@ export const sidebarConfig: { // @public @deprecated export const SidebarContext: React_2.Context; -// @public -export const SidebarContextProvider: ({ - children, - value, -}: { - children: ReactNode; - value: SidebarContextType; -}) => JSX.Element; - -// @public +// @public @deprecated export type SidebarContextType = { isOpen: boolean; setOpen: (open: boolean) => void; @@ -987,6 +978,21 @@ export type SidebarItemClassKey = | 'arrows' | 'selected'; +// @public +export type SidebarOpenState = { + isOpen: boolean; + setOpen: (open: boolean) => void; +}; + +// @public +export const SidebarOpenStateProvider: ({ + children, + value, +}: { + children: ReactNode; + value: SidebarOpenState; +}) => JSX.Element; + // @public (undocumented) export type SidebarOptions = { drawerWidthClosed?: number; @@ -1008,11 +1014,25 @@ export type SidebarPageProps = { children?: React_2.ReactNode; }; +// @public +export type SidebarPinState = { + isPinned: boolean; + toggleSidebarPinState: () => any; + isMobile?: boolean; +}; + // @public @deprecated export const SidebarPinStateContext: React_2.Context; +// @public @deprecated +export type SidebarPinStateContextType = { + isPinned: boolean; + toggleSidebarPinState: () => any; + isMobile?: boolean; +}; + // @public -export const SidebarPinStateContextProvider: ({ +export const SidebarPinStateProvider: ({ children, value, }: { @@ -1020,13 +1040,6 @@ export const SidebarPinStateContextProvider: ({ value: SidebarPinStateContextType; }) => JSX.Element; -// @public -export type SidebarPinStateContextType = { - isPinned: boolean; - toggleSidebarPinState: () => any; - isMobile?: boolean; -}; - // @public (undocumented) export type SidebarProps = { openDelayMs?: number; @@ -1461,10 +1474,10 @@ export class UserIdentity implements IdentityApi { } // @public -export const useSidebar: () => SidebarContextType; +export const useSidebarOpenState: () => SidebarOpenState; // @public -export const useSidebarPinState: () => SidebarPinStateContextType; +export const useSidebarPinState: () => SidebarPinState; // Warning: (ae-missing-release-tag) "useSupportConfig" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) // diff --git a/packages/core-components/src/layout/Sidebar/Bar.test.tsx b/packages/core-components/src/layout/Sidebar/Bar.test.tsx index ec9c08a1eb..87e62f15e0 100644 --- a/packages/core-components/src/layout/Sidebar/Bar.test.tsx +++ b/packages/core-components/src/layout/Sidebar/Bar.test.tsx @@ -27,14 +27,14 @@ import { SidebarExpandButton, SidebarItem, SidebarSearchField, - SidebarPinStateContextProvider, + SidebarPinStateProvider, SidebarSubmenu, SidebarSubmenuItem, } from '.'; async function renderScalableSidebar() { await renderInTestApp( - - , + , ); } diff --git a/packages/core-components/src/layout/Sidebar/Bar.tsx b/packages/core-components/src/layout/Sidebar/Bar.tsx index a315406b58..c86f60a0a2 100644 --- a/packages/core-components/src/layout/Sidebar/Bar.tsx +++ b/packages/core-components/src/layout/Sidebar/Bar.tsx @@ -32,7 +32,7 @@ import { } from './config'; import { BackstageTheme } from '@backstage/theme'; import { useContent } from './Page'; -import { SidebarContextProvider } from './SidebarContext'; +import { SidebarOpenStateProvider } from './SidebarOpenStateContext'; import { useSidebarPinState } from './SidebarPinStateContext'; import { MobileSidebar } from './MobileSidebar'; @@ -190,7 +190,7 @@ const DesktopSidebar = (props: DesktopSidebarProps) => { return (
- + ); }; diff --git a/packages/core-components/src/layout/Sidebar/Intro.tsx b/packages/core-components/src/layout/Sidebar/Intro.tsx index 51d453b22d..05d580c960 100644 --- a/packages/core-components/src/layout/Sidebar/Intro.tsx +++ b/packages/core-components/src/layout/Sidebar/Intro.tsx @@ -28,7 +28,7 @@ import { SIDEBAR_INTRO_LOCAL_STORAGE, } from './config'; import { SidebarDivider } from './Items'; -import { useSidebar } from './SidebarContext'; +import { useSidebarOpenState } from './SidebarOpenStateContext'; /** @public */ export type SidebarIntroClassKey = @@ -151,7 +151,7 @@ const recentlyViewedIntroText = 'And your recently viewed plugins will pop up here!'; export function SidebarIntro(_props: {}) { - const { isOpen } = useSidebar(); + const { isOpen } = useSidebarOpenState(); const defaultValue = { starredItemsDismissed: false, recentlyViewedItemsDismissed: false, diff --git a/packages/core-components/src/layout/Sidebar/Items.tsx b/packages/core-components/src/layout/Sidebar/Items.tsx index 3d294a54a8..0413864084 100644 --- a/packages/core-components/src/layout/Sidebar/Items.tsx +++ b/packages/core-components/src/layout/Sidebar/Items.tsx @@ -61,7 +61,7 @@ import DoubleArrowLeft from './icons/DoubleArrowLeft'; import DoubleArrowRight from './icons/DoubleArrowRight'; import { isLocationMatch } from './utils'; import { Location } from 'history'; -import { useSidebar } from './SidebarContext'; +import { useSidebarOpenState } from './SidebarOpenStateContext'; /** @public */ export type SidebarItemClassKey = @@ -369,7 +369,7 @@ const SidebarItemBase = forwardRef((props, ref) => { // XXX (@koroeskohr): unsure this is optimal. But I just really didn't want to have the item component // depend on the current location, and at least have it being optionally forced to selected. // Still waiting on a Q answered to fine tune the implementation - const { isOpen } = useSidebar(); + const { isOpen } = useSidebarOpenState(); const divStyle = !isOpen && hasSubmenu ? { display: 'flex', marginLeft: '24px' } : {}; @@ -671,7 +671,7 @@ export const SidebarScrollWrapper = styled('div')(({ theme }) => { export const SidebarExpandButton = () => { const { sidebarConfig } = useContext(SidebarConfigContext); const classes = useMemoStyles(sidebarConfig); - const { isOpen, setOpen } = useSidebar(); + const { isOpen, setOpen } = useSidebarOpenState(); const isSmallScreen = useMediaQuery( theme => theme.breakpoints.down('md'), { noSsr: true }, diff --git a/packages/core-components/src/layout/Sidebar/MobileSidebar.tsx b/packages/core-components/src/layout/Sidebar/MobileSidebar.tsx index 46f38d9a2c..b51e3cbc0d 100644 --- a/packages/core-components/src/layout/Sidebar/MobileSidebar.tsx +++ b/packages/core-components/src/layout/Sidebar/MobileSidebar.tsx @@ -27,7 +27,7 @@ import MenuIcon from '@material-ui/icons/Menu'; import { orderBy } from 'lodash'; import React, { createContext, useEffect, useState, useContext } from 'react'; import { useLocation } from 'react-router'; -import { SidebarContextProvider } from './SidebarContext'; +import { SidebarOpenStateProvider } from './SidebarOpenStateContext'; import { SidebarGroup } from './SidebarGroup'; import { SidebarConfigContext, SidebarConfig } from './config'; @@ -208,7 +208,7 @@ export const MobileSidebar = (props: MobileSidebarProps) => { !sidebarGroups[selectedMenuItemIndex].props.to; return ( - {} }}> + {} }}> @@ -232,6 +232,6 @@ export const MobileSidebar = (props: MobileSidebarProps) => { {sidebarGroups} - + ); }; diff --git a/packages/core-components/src/layout/Sidebar/Page.tsx b/packages/core-components/src/layout/Sidebar/Page.tsx index 17cd5ddee1..bdf41b5dd7 100644 --- a/packages/core-components/src/layout/Sidebar/Page.tsx +++ b/packages/core-components/src/layout/Sidebar/Page.tsx @@ -29,7 +29,7 @@ import { SidebarConfigContext, SidebarConfig } from './config'; import { BackstageTheme } from '@backstage/theme'; import { LocalStorage } from './localStorage'; import useMediaQuery from '@material-ui/core/useMediaQuery'; -import { SidebarPinStateContextProvider } from './SidebarPinStateContext'; +import { SidebarPinStateProvider } from './SidebarPinStateContext'; export type SidebarPageClassKey = 'root'; @@ -114,7 +114,7 @@ export function SidebarPage(props: SidebarPageProps) { const classes = useStyles({ isPinned, sidebarConfig }); return ( -
{props.children}
- + ); } diff --git a/packages/core-components/src/layout/Sidebar/SidebarContext.test.tsx b/packages/core-components/src/layout/Sidebar/SidebarOpenStateContext.test.tsx similarity index 78% rename from packages/core-components/src/layout/Sidebar/SidebarContext.test.tsx rename to packages/core-components/src/layout/Sidebar/SidebarOpenStateContext.test.tsx index 43a18851d7..4953e1341b 100644 --- a/packages/core-components/src/layout/Sidebar/SidebarContext.test.tsx +++ b/packages/core-components/src/layout/Sidebar/SidebarOpenStateContext.test.tsx @@ -19,17 +19,17 @@ import { waitFor } from '@testing-library/react'; import { renderHook, act } from '@testing-library/react-hooks'; import { LegacySidebarContext, - SidebarContextProvider, - useSidebar, -} from './SidebarContext'; + SidebarOpenStateProvider, + useSidebarOpenState, +} from './SidebarOpenStateContext'; -describe('SidebarContext', () => { - describe('SidebarContextProvider', () => { +describe('SidebarOpenStateContext', () => { + describe('SidebarOpenStateProvider', () => { it('should render children', async () => { const { findByText } = await renderWithEffects( - {} }}> + {} }}> Child - , + , ); expect(await findByText('Child')).toBeInTheDocument(); }); @@ -41,23 +41,23 @@ describe('SidebarContext', () => { }; const { findByText } = await renderWithEffects( - {}, }} > - , + , ); expect(await findByText('true')).toBeInTheDocument(); }); }); - describe('useSidebar', () => { + describe('useSidebarOpenState', () => { it('does not need to be invoked within provider', () => { - const { result } = renderHook(() => useSidebar()); + const { result } = renderHook(() => useSidebarOpenState()); expect(result.current.isOpen).toBe(false); expect(typeof result.current.setOpen).toBe('function'); }); @@ -65,7 +65,7 @@ describe('SidebarContext', () => { it('should read and update state', async () => { let actualValue = true; const wrapper = ({ children }: { children: ReactNode }) => ( - { @@ -74,9 +74,11 @@ describe('SidebarContext', () => { }} > {children} - + ); - const { result, rerender } = renderHook(() => useSidebar(), { wrapper }); + const { result, rerender } = renderHook(() => useSidebarOpenState(), { + wrapper, + }); expect(result.current.isOpen).toBe(true); diff --git a/packages/core-components/src/layout/Sidebar/SidebarContext.tsx b/packages/core-components/src/layout/Sidebar/SidebarOpenStateContext.tsx similarity index 79% rename from packages/core-components/src/layout/Sidebar/SidebarContext.tsx rename to packages/core-components/src/layout/Sidebar/SidebarOpenStateContext.tsx index 4e47b6040a..1ceb6d95cf 100644 --- a/packages/core-components/src/layout/Sidebar/SidebarContext.tsx +++ b/packages/core-components/src/layout/Sidebar/SidebarOpenStateContext.tsx @@ -23,13 +23,24 @@ import { /** * Types for the `SidebarContext` * - * @public + * @public @deprecated + * Use `SidebarOpenState` instead. */ export type SidebarContextType = { isOpen: boolean; setOpen: (open: boolean) => void; }; +/** + * The open state of the sidebar. + * + * @public + */ +export type SidebarOpenState = { + isOpen: boolean; + setOpen: (open: boolean) => void; +}; + const defaultSidebarContext = { isOpen: false, setOpen: () => {}, @@ -46,20 +57,20 @@ export const LegacySidebarContext = createContext( ); const VersionedSidebarContext = createVersionedContext<{ - 1: SidebarContextType; -}>('sidebar-context'); + 1: SidebarOpenState; +}>('sidebar-open-state-context'); /** * Provides context for reading and updating sidebar state. * * @public */ -export const SidebarContextProvider = ({ +export const SidebarOpenStateProvider = ({ children, value, }: { children: ReactNode; - value: SidebarContextType; + value: SidebarOpenState; }) => ( { +export const useSidebarOpenState = (): SidebarOpenState => { const versionedSidebarContext = useContext(VersionedSidebarContext); - // Invoked from outside a SidbarContextProvider, return a default value. + // Invoked from outside a SidebarOpenStateProvider, return a default value. if (versionedSidebarContext === undefined) { return defaultSidebarContext; } diff --git a/packages/core-components/src/layout/Sidebar/SidebarPinStateContext.test.tsx b/packages/core-components/src/layout/Sidebar/SidebarPinStateContext.test.tsx index d8bc054e4a..1a86ec612e 100644 --- a/packages/core-components/src/layout/Sidebar/SidebarPinStateContext.test.tsx +++ b/packages/core-components/src/layout/Sidebar/SidebarPinStateContext.test.tsx @@ -19,15 +19,15 @@ import { waitFor } from '@testing-library/react'; import { renderHook, act } from '@testing-library/react-hooks'; import { LegacySidebarPinStateContext, - SidebarPinStateContextProvider, + SidebarPinStateProvider, useSidebarPinState, } from './SidebarPinStateContext'; describe('SidebarPinStateContext', () => { - describe('SidebarPinStateContextProvider', () => { + describe('SidebarPinStateProvider', () => { it('should render children', async () => { const { findByText } = await renderWithEffects( - { }} > Child - , + , ); expect(await findByText('Child')).toBeInTheDocument(); }); @@ -47,7 +47,7 @@ describe('SidebarPinStateContext', () => { }; const { findByText } = await renderWithEffects( - { }} > - , + , ); expect(await findByText('true')).toBeInTheDocument(); @@ -73,7 +73,7 @@ describe('SidebarPinStateContext', () => { it('should read and update state', async () => { let actualValue = true; const wrapper = ({ children }: { children: ReactNode }) => ( - { }} > {children} - + ); const { result, rerender } = renderHook(() => useSidebarPinState(), { wrapper, diff --git a/packages/core-components/src/layout/Sidebar/SidebarPinStateContext.tsx b/packages/core-components/src/layout/Sidebar/SidebarPinStateContext.tsx index ec38884890..0eb59dd119 100644 --- a/packages/core-components/src/layout/Sidebar/SidebarPinStateContext.tsx +++ b/packages/core-components/src/layout/Sidebar/SidebarPinStateContext.tsx @@ -22,7 +22,8 @@ import React, { createContext, ReactNode, useContext } from 'react'; /** * Type of `SidebarPinStateContext` * - * @public + * @public @deprecated + * Use `SidebarPinState` instead. */ export type SidebarPinStateContextType = { isPinned: boolean; @@ -30,6 +31,17 @@ export type SidebarPinStateContextType = { isMobile?: boolean; }; +/** + * The pin state of the sidebar. + * + * @public + */ +export type SidebarPinState = { + isPinned: boolean; + toggleSidebarPinState: () => any; + isMobile?: boolean; +}; + const defaultSidebarPinStateContext = { isPinned: true, toggleSidebarPinState: () => {}, @@ -46,7 +58,7 @@ export const LegacySidebarPinStateContext = createContext(defaultSidebarPinStateContext); const VersionedSidebarPinStateContext = createVersionedContext<{ - 1: SidebarPinStateContextType; + 1: SidebarPinState; }>('sidebar-pin-state-context'); /** @@ -54,7 +66,7 @@ const VersionedSidebarPinStateContext = createVersionedContext<{ * * @public */ -export const SidebarPinStateContextProvider = ({ +export const SidebarPinStateProvider = ({ children, value, }: { @@ -75,10 +87,10 @@ export const SidebarPinStateContextProvider = ({ * * @public */ -export const useSidebarPinState = (): SidebarPinStateContextType => { +export const useSidebarPinState = (): SidebarPinState => { const versionedSidebarContext = useContext(VersionedSidebarPinStateContext); - // Invoked from outside a SidebarPinStateContextProvider: default value. + // Invoked from outside a SidebarPinStateProvider: default value. if (versionedSidebarContext === undefined) { return defaultSidebarPinStateContext; } diff --git a/packages/core-components/src/layout/Sidebar/SidebarSubmenu.tsx b/packages/core-components/src/layout/Sidebar/SidebarSubmenu.tsx index d8a87c98d9..438e580ef4 100644 --- a/packages/core-components/src/layout/Sidebar/SidebarSubmenu.tsx +++ b/packages/core-components/src/layout/Sidebar/SidebarSubmenu.tsx @@ -22,7 +22,7 @@ import { SidebarConfigContext, SubmenuConfig, } from './config'; -import { useSidebar } from './SidebarContext'; +import { useSidebarOpenState } from './SidebarOpenStateContext'; import { BackstageTheme } from '@backstage/theme'; const useStyles = makeStyles< @@ -105,7 +105,7 @@ export type SidebarSubmenuProps = { * @public */ export const SidebarSubmenu = (props: SidebarSubmenuProps) => { - const { isOpen } = useSidebar(); + const { isOpen } = useSidebarOpenState(); const { sidebarConfig, submenuConfig } = useContext(SidebarConfigContext); const left = isOpen ? sidebarConfig.drawerWidthOpen diff --git a/packages/core-components/src/layout/Sidebar/index.ts b/packages/core-components/src/layout/Sidebar/index.ts index 06681ded52..4c91ea6a8f 100644 --- a/packages/core-components/src/layout/Sidebar/index.ts +++ b/packages/core-components/src/layout/Sidebar/index.ts @@ -50,13 +50,19 @@ export { SIDEBAR_INTRO_LOCAL_STORAGE, sidebarConfig } from './config'; export type { SidebarOptions, SubmenuOptions } from './config'; export { LegacySidebarContext as SidebarContext, - SidebarContextProvider, - useSidebar, -} from './SidebarContext'; -export type { SidebarContextType } from './SidebarContext'; + SidebarOpenStateProvider, + useSidebarOpenState, +} from './SidebarOpenStateContext'; +export type { + SidebarContextType, + SidebarOpenState, +} from './SidebarOpenStateContext'; export { LegacySidebarPinStateContext as SidebarPinStateContext, - SidebarPinStateContextProvider, + SidebarPinStateProvider, useSidebarPinState, } from './SidebarPinStateContext'; -export type { SidebarPinStateContextType } from './SidebarPinStateContext'; +export type { + SidebarPinStateContextType, + SidebarPinState, +} from './SidebarPinStateContext'; diff --git a/packages/create-app/templates/default-app/packages/app/src/components/Root/Root.tsx b/packages/create-app/templates/default-app/packages/app/src/components/Root/Root.tsx index 05e11458ce..b1164a32f0 100644 --- a/packages/create-app/templates/default-app/packages/app/src/components/Root/Root.tsx +++ b/packages/create-app/templates/default-app/packages/app/src/components/Root/Root.tsx @@ -38,7 +38,7 @@ import { SidebarPage, SidebarScrollWrapper, SidebarSpace, - useSidebar, + useSidebarOpenState, } from '@backstage/core-components'; import MenuIcon from '@material-ui/icons/Menu'; import SearchIcon from '@material-ui/icons/Search'; @@ -60,7 +60,7 @@ const useSidebarLogoStyles = makeStyles({ const SidebarLogo = () => { const classes = useSidebarLogoStyles(); - const { isOpen } = useSidebar(); + const { isOpen } = useSidebarOpenState(); return (
diff --git a/packages/techdocs-cli-embedded-app/src/components/Root/Root.tsx b/packages/techdocs-cli-embedded-app/src/components/Root/Root.tsx index 6613e7c0ea..cd8b5d35ed 100644 --- a/packages/techdocs-cli-embedded-app/src/components/Root/Root.tsx +++ b/packages/techdocs-cli-embedded-app/src/components/Root/Root.tsx @@ -27,7 +27,7 @@ import { SidebarPage, sidebarConfig, SidebarDivider, - useSidebar, + useSidebarOpenState, } from '@backstage/core-components'; import { NavLink } from 'react-router-dom'; @@ -48,7 +48,7 @@ const useSidebarLogoStyles = makeStyles({ const SidebarLogo = () => { const classes = useSidebarLogoStyles(); - const { isOpen } = useSidebar(); + const { isOpen } = useSidebarOpenState(); return (
diff --git a/plugins/shortcuts/src/ShortcutItem.test.tsx b/plugins/shortcuts/src/ShortcutItem.test.tsx index d1a1018513..b98dd2dc0b 100644 --- a/plugins/shortcuts/src/ShortcutItem.test.tsx +++ b/plugins/shortcuts/src/ShortcutItem.test.tsx @@ -20,7 +20,7 @@ import { ShortcutItem } from './ShortcutItem'; import { Shortcut } from './types'; import { LocalStoredShortcuts } from './api'; import { MockStorageApi, renderInTestApp } from '@backstage/test-utils'; -import { SidebarContextProvider } from '@backstage/core-components'; +import { SidebarOpenStateProvider } from '@backstage/core-components'; describe('ShortcutItem', () => { const shortcut: Shortcut = { @@ -32,9 +32,9 @@ describe('ShortcutItem', () => { it('displays the shortcut', async () => { await renderInTestApp( - {} }}> + {} }}> - , + , ); expect(screen.getByText('ST')).toBeInTheDocument(); expect(screen.getByText('some title')).toBeInTheDocument(); diff --git a/plugins/shortcuts/src/Shortcuts.test.tsx b/plugins/shortcuts/src/Shortcuts.test.tsx index 813dbb8def..02b351d3a4 100644 --- a/plugins/shortcuts/src/Shortcuts.test.tsx +++ b/plugins/shortcuts/src/Shortcuts.test.tsx @@ -24,12 +24,12 @@ import { screen, waitFor } from '@testing-library/react'; import { Shortcuts } from './Shortcuts'; import { LocalStoredShortcuts, shortcutsApiRef } from './api'; -import { SidebarContextProvider } from '@backstage/core-components'; +import { SidebarOpenStateProvider } from '@backstage/core-components'; describe('Shortcuts', () => { it('displays an add button', async () => { await renderInTestApp( - {} }}> + {} }}> { > - , + , ); await waitFor(() => !screen.queryByTestId('progress')); expect(screen.getByText('Add Shortcuts')).toBeInTheDocument(); diff --git a/plugins/user-settings/src/components/General/UserSettingsPinToggle.test.tsx b/plugins/user-settings/src/components/General/UserSettingsPinToggle.test.tsx index a41792768a..ef8046a54e 100644 --- a/plugins/user-settings/src/components/General/UserSettingsPinToggle.test.tsx +++ b/plugins/user-settings/src/components/General/UserSettingsPinToggle.test.tsx @@ -18,14 +18,14 @@ import { renderWithEffects, wrapInTestApp } from '@backstage/test-utils'; import { fireEvent } from '@testing-library/react'; import React from 'react'; import { UserSettingsPinToggle } from './UserSettingsPinToggle'; -import { SidebarPinStateContextProvider } from '@backstage/core-components'; +import { SidebarPinStateProvider } from '@backstage/core-components'; describe('', () => { it('toggles the pin sidebar button', async () => { const mockToggleFn = jest.fn(); const rendered = await renderWithEffects( wrapInTestApp( - ', () => { }} > - , + , ), ); expect(rendered.getByText('Pin Sidebar')).toBeInTheDocument(); From 58a957f4fc6faafc47e8f890e7aaad3b0ea1ff73 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Mon, 23 May 2022 14:37:39 +0200 Subject: [PATCH 6/7] Better document hooks, props, etc. Signed-off-by: Eric Peterson --- .../layout/Sidebar/SidebarOpenStateContext.tsx | 12 +++++++++++- .../layout/Sidebar/SidebarPinStateContext.tsx | 17 ++++++++++++++++- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/packages/core-components/src/layout/Sidebar/SidebarOpenStateContext.tsx b/packages/core-components/src/layout/Sidebar/SidebarOpenStateContext.tsx index 1ceb6d95cf..be15785f29 100644 --- a/packages/core-components/src/layout/Sidebar/SidebarOpenStateContext.tsx +++ b/packages/core-components/src/layout/Sidebar/SidebarOpenStateContext.tsx @@ -37,7 +37,16 @@ export type SidebarContextType = { * @public */ export type SidebarOpenState = { + /** + * Whether or not the sidebar is open and full-width. When `false`, the + * sidebar is "closed" and typically only shows icons with no text. + */ isOpen: boolean; + + /** + * A function to set whether or not the sidebar is open. Pass `true` to open + * the sidebar. Pass `false` to close it. + */ setOpen: (open: boolean) => void; }; @@ -82,7 +91,8 @@ export const SidebarOpenStateProvider = ({ ); /** - * Hook to read and update the sidebar's open state. + * Hook to read and update the sidebar's open state, which controls whether or + * not the sidebar is open and full-width, or closed and only displaying icons. * * @public */ diff --git a/packages/core-components/src/layout/Sidebar/SidebarPinStateContext.tsx b/packages/core-components/src/layout/Sidebar/SidebarPinStateContext.tsx index 0eb59dd119..a85929ab70 100644 --- a/packages/core-components/src/layout/Sidebar/SidebarPinStateContext.tsx +++ b/packages/core-components/src/layout/Sidebar/SidebarPinStateContext.tsx @@ -37,8 +37,22 @@ export type SidebarPinStateContextType = { * @public */ export type SidebarPinState = { + /** + * Whether or not the sidebar is pinned to the `open` state. When `isPinned` + * is `false`, the sidebar opens and closes on hover. When `true`, the + * sidebar is permanently opened, regardless of user interaction. + */ isPinned: boolean; + + /** + * A function to toggle the pin state of the sidebar. + */ toggleSidebarPinState: () => any; + + /** + * Whether or not the sidebar is or should be rendered in a mobile-optimized + * way. + */ isMobile?: boolean; }; @@ -83,7 +97,8 @@ export const SidebarPinStateProvider = ({ ); /** - * Hook to read and update sidebar pin state. + * Hook to read and update sidebar pin state, which controls whether or not the + * sidebar is pinned open. * * @public */ From 566e26668347a8d13ccdfaafd31653561b546fbc Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Mon, 30 May 2022 19:19:28 +0200 Subject: [PATCH 7/7] Consume legacy context in hook too, just in case. Signed-off-by: Eric Peterson --- .../Sidebar/SidebarOpenStateContext.test.tsx | 20 +++++++++++++++++ .../Sidebar/SidebarOpenStateContext.tsx | 20 +++++++++-------- .../Sidebar/SidebarPinStateContext.test.tsx | 22 +++++++++++++++++++ .../layout/Sidebar/SidebarPinStateContext.tsx | 16 ++++++++------ 4 files changed, 62 insertions(+), 16 deletions(-) diff --git a/packages/core-components/src/layout/Sidebar/SidebarOpenStateContext.test.tsx b/packages/core-components/src/layout/Sidebar/SidebarOpenStateContext.test.tsx index 4953e1341b..f1900f025c 100644 --- a/packages/core-components/src/layout/Sidebar/SidebarOpenStateContext.test.tsx +++ b/packages/core-components/src/layout/Sidebar/SidebarOpenStateContext.test.tsx @@ -56,6 +56,26 @@ describe('SidebarOpenStateContext', () => { }); describe('useSidebarOpenState', () => { + it('can be invoked within legacy context', () => { + const wrapper = ({ children }: { children: ReactNode }) => ( + {}, + }} + > + {children} + + ); + + const { result } = renderHook(() => useSidebarOpenState(), { + wrapper, + }); + + expect(result.current.isOpen).toBe(true); + expect(typeof result.current.setOpen).toBe('function'); + }); + it('does not need to be invoked within provider', () => { const { result } = renderHook(() => useSidebarOpenState()); expect(result.current.isOpen).toBe(false); diff --git a/packages/core-components/src/layout/Sidebar/SidebarOpenStateContext.tsx b/packages/core-components/src/layout/Sidebar/SidebarOpenStateContext.tsx index be15785f29..635306959a 100644 --- a/packages/core-components/src/layout/Sidebar/SidebarOpenStateContext.tsx +++ b/packages/core-components/src/layout/Sidebar/SidebarOpenStateContext.tsx @@ -50,7 +50,7 @@ export type SidebarOpenState = { setOpen: (open: boolean) => void; }; -const defaultSidebarContext = { +const defaultSidebarOpenStateContext = { isOpen: false, setOpen: () => {}, }; @@ -62,7 +62,7 @@ const defaultSidebarContext = { * Use `` + `useSidebar()` instead. */ export const LegacySidebarContext = createContext( - defaultSidebarContext, + defaultSidebarOpenStateContext, ); const VersionedSidebarContext = createVersionedContext<{ @@ -97,17 +97,19 @@ export const SidebarOpenStateProvider = ({ * @public */ export const useSidebarOpenState = (): SidebarOpenState => { - const versionedSidebarContext = useContext(VersionedSidebarContext); + const versionedOpenStateContext = useContext(VersionedSidebarContext); + const legacyOpenStateContext = useContext(LegacySidebarContext); - // Invoked from outside a SidebarOpenStateProvider, return a default value. - if (versionedSidebarContext === undefined) { - return defaultSidebarContext; + // Invoked from outside a SidebarOpenStateProvider: check for the legacy + // context's value, but otherwise return the default. + if (versionedOpenStateContext === undefined) { + return legacyOpenStateContext || defaultSidebarOpenStateContext; } - const sidebarContext = versionedSidebarContext.atVersion(1); - if (sidebarContext === undefined) { + const openStateContext = versionedOpenStateContext.atVersion(1); + if (openStateContext === undefined) { throw new Error('No context found for version 1.'); } - return sidebarContext; + return openStateContext; }; diff --git a/packages/core-components/src/layout/Sidebar/SidebarPinStateContext.test.tsx b/packages/core-components/src/layout/Sidebar/SidebarPinStateContext.test.tsx index 1a86ec612e..8bd1597725 100644 --- a/packages/core-components/src/layout/Sidebar/SidebarPinStateContext.test.tsx +++ b/packages/core-components/src/layout/Sidebar/SidebarPinStateContext.test.tsx @@ -63,6 +63,28 @@ describe('SidebarPinStateContext', () => { }); describe('useSidebarPinState', () => { + it('can be invoked within legacy context', () => { + const wrapper = ({ children }: { children: ReactNode }) => ( + {}, + }} + > + {children} + + ); + + const { result } = renderHook(() => useSidebarPinState(), { + wrapper, + }); + + expect(result.current.isPinned).toBe(true); + expect(result.current.isMobile).toBe(true); + expect(typeof result.current.toggleSidebarPinState).toBe('function'); + }); + it('does not need to be invoked within provider', () => { const { result } = renderHook(() => useSidebarPinState()); expect(result.current.isPinned).toBe(true); diff --git a/packages/core-components/src/layout/Sidebar/SidebarPinStateContext.tsx b/packages/core-components/src/layout/Sidebar/SidebarPinStateContext.tsx index a85929ab70..e00ed259e9 100644 --- a/packages/core-components/src/layout/Sidebar/SidebarPinStateContext.tsx +++ b/packages/core-components/src/layout/Sidebar/SidebarPinStateContext.tsx @@ -103,17 +103,19 @@ export const SidebarPinStateProvider = ({ * @public */ export const useSidebarPinState = (): SidebarPinState => { - const versionedSidebarContext = useContext(VersionedSidebarPinStateContext); + const versionedPinStateContext = useContext(VersionedSidebarPinStateContext); + const legacyPinStateContext = useContext(LegacySidebarPinStateContext); - // Invoked from outside a SidebarPinStateProvider: default value. - if (versionedSidebarContext === undefined) { - return defaultSidebarPinStateContext; + // Invoked from outside a SidebarPinStateProvider: check for the legacy + // context's value, but otherwise return the default. + if (versionedPinStateContext === undefined) { + return legacyPinStateContext || defaultSidebarPinStateContext; } - const sidebarContext = versionedSidebarContext.atVersion(1); - if (sidebarContext === undefined) { + const pinStateContext = versionedPinStateContext.atVersion(1); + if (pinStateContext === undefined) { throw new Error('No context found for version 1.'); } - return sidebarContext; + return pinStateContext; };