diff --git a/.changeset/live-nav-rest-items.md b/.changeset/live-nav-rest-items.md new file mode 100644 index 0000000000..41b0fd8636 --- /dev/null +++ b/.changeset/live-nav-rest-items.md @@ -0,0 +1,6 @@ +--- +'@backstage/plugin-app': patch +'@backstage/plugin-app-react': patch +--- + +`NavContentBlueprint` nav item collections now keep previously collected `rest()` results in sync when additional items are taken later in the same render, making it easier to place items across multiple sidebar sections. diff --git a/packages/app/src/modules/appModuleNav.tsx b/packages/app/src/modules/appModuleNav.tsx index a507679c73..6c6ba0bfb6 100644 --- a/packages/app/src/modules/appModuleNav.tsx +++ b/packages/app/src/modules/appModuleNav.tsx @@ -27,15 +27,11 @@ import { } from '@backstage/core-components'; import SearchIcon from '@material-ui/icons/Search'; import MenuIcon from '@material-ui/icons/Menu'; -import BuildIcon from '@material-ui/icons/Build'; import { createFrontendModule } from '@backstage/frontend-plugin-api'; import { NavContentBlueprint } from '@backstage/plugin-app-react'; import { SidebarSearchModal } from '@backstage/plugin-search'; import { NotificationsSidebarItem } from '@backstage/plugin-notifications'; -import { - Settings, - UserSettingsSignInAvatar, -} from '@backstage/plugin-user-settings'; +import { UserSettingsSignInAvatar } from '@backstage/plugin-user-settings'; import { makeStyles } from '@material-ui/core/styles'; const useSidebarLogoStyles = makeStyles({ @@ -111,7 +107,9 @@ export const appModuleNav = createFrontendModule({ text={item.title} /> )); - nav.take('page:home'); // Skip home + // Skip these + nav.take('page:home'); + nav.take('page:search'); return ( @@ -136,8 +134,8 @@ export const appModuleNav = createFrontendModule({ to="/settings" > - - + {nav.take('page:devtools')} + {nav.take('page:user-settings')} ); diff --git a/plugins/app-react/src/blueprints/NavContentBlueprint.test.tsx b/plugins/app-react/src/blueprints/NavContentBlueprint.test.tsx index 2c74300d9d..eb0e8c1007 100644 --- a/plugins/app-react/src/blueprints/NavContentBlueprint.test.tsx +++ b/plugins/app-react/src/blueprints/NavContentBlueprint.test.tsx @@ -31,36 +31,72 @@ function mockNode(id: string): AppNode { function mockNavItems(items: NavContentNavItem[]): NavContentNavItems { const taken = new Set(); + let restItems: NavContentNavItem[] | undefined; return { take(id: string) { const item = items.find(i => i.node.spec.id === id); - if (item) { + if (item && !taken.has(id)) { taken.add(id); + if (restItems) { + const index = restItems.findIndex( + restItem => restItem.node.spec.id === id, + ); + if (index !== -1) { + restItems.splice(index, 1); + } + } } return item; }, - rest: () => items.filter(i => !taken.has(i.node.spec.id)), + rest: () => { + if (!restItems) { + restItems = items.filter(i => !taken.has(i.node.spec.id)); + } + return restItems; + }, clone() { return mockNavItems(items); }, withComponent(Component: (props: NavContentNavItem) => JSX.Element) { + let renderedItems: JSX.Element[] | undefined; return { take: (id: string) => { const item = items.find(i => i.node.spec.id === id); - if (item) { + if (item && !taken.has(id)) { taken.add(id); - return ; + if (restItems) { + const index = restItems.findIndex( + restItem => restItem.node.spec.id === id, + ); + if (index !== -1) { + restItems.splice(index, 1); + } + } + if (renderedItems) { + const index = renderedItems.findIndex( + renderedItem => renderedItem.key === item.node.spec.id, + ); + if (index !== -1) { + renderedItems.splice(index, 1); + } + } + return ; } return null; }, rest: (options?: { sortBy?: 'title' }) => { - const remaining = items.filter(i => !taken.has(i.node.spec.id)); - if (options?.sortBy === 'title') { - remaining.sort((a, b) => a.title.localeCompare(b.title)); + if (!restItems) { + restItems = items.filter(i => !taken.has(i.node.spec.id)); } - return remaining.map(item => ( - - )); + if (!renderedItems) { + if (options?.sortBy === 'title') { + restItems.sort((a, b) => a.title.localeCompare(b.title)); + } + renderedItems = restItems.map(item => ( + + )); + } + return renderedItems; }, }; }, @@ -267,4 +303,61 @@ describe('NavContentBlueprint', () => { expect(docsLink).toBeInTheDocument(); expect(docsLink.closest('nav')).toBeTruthy(); }); + + it('should keep the rendered rest array in sync with later take calls', () => { + const items: NavContentNavItem[] = [ + { + node: mockNode('page:catalog'), + href: '/catalog', + title: 'Catalog', + icon: catalog, + routeRef, + }, + { + node: mockNode('page:devtools'), + href: '/devtools', + title: 'DevTools', + icon: devtools, + routeRef, + }, + { + node: mockNode('page:user-settings'), + href: '/settings', + title: 'Settings', + icon: settings, + routeRef, + }, + ]; + + const extension = NavContentBlueprint.make({ + name: 'test', + params: { + component: ({ navItems }) => { + const nav = navItems.withComponent(item => ( + {item.title} + )); + const rest = nav.rest({ sortBy: 'title' }); + + return ( +
+ + +
{nav.take('page:user-settings')}
+
+ ); + }, + }, + }); + + const tester = createExtensionTester(extension); + const Component = tester.get(NavContentBlueprint.dataRefs.component); + + render(); + + expect(screen.getByText('Catalog').closest('nav')).toBeTruthy(); + expect(screen.queryByText('DevTools')?.closest('nav')).toBeFalsy(); + expect(screen.queryByText('Settings')?.closest('nav')).toBeFalsy(); + expect(screen.getByText('DevTools').closest('aside')).toBeTruthy(); + expect(screen.getByText('Settings').closest('footer')).toBeTruthy(); + }); }); diff --git a/plugins/app-react/src/blueprints/NavContentBlueprint.ts b/plugins/app-react/src/blueprints/NavContentBlueprint.ts index f88ba48a38..8cfbf7a276 100644 --- a/plugins/app-react/src/blueprints/NavContentBlueprint.ts +++ b/plugins/app-react/src/blueprints/NavContentBlueprint.ts @@ -53,7 +53,12 @@ export interface NavContentNavItem { export interface NavContentNavItemsWithComponent { /** Render and take a specific item by extension ID. Returns null if not found. */ take(id: string): JSX.Element | null; - /** Render all remaining items not yet taken, optionally sorted. */ + /** + * Render all remaining items not yet taken, optionally sorted. + * + * The returned array is live and is updated when later `take` calls remove + * items from the collection. + */ rest(options?: { sortBy?: 'title' }): JSX.Element[]; } @@ -66,7 +71,12 @@ export interface NavContentNavItemsWithComponent { export interface NavContentNavItems { /** Take an item by extension ID, removing it from the collection. */ take(id: string): NavContentNavItem | undefined; - /** All items not yet taken. */ + /** + * All items not yet taken. + * + * The returned array is live and is updated when later `take` calls remove + * items from the collection. + */ rest(): NavContentNavItem[]; /** Create a copy of the collection preserving the current taken state. */ clone(): NavContentNavItems; diff --git a/plugins/app/src/extensions/AppNav.tsx b/plugins/app/src/extensions/AppNav.tsx index 129a94cd9e..09203303e5 100644 --- a/plugins/app/src/extensions/AppNav.tsx +++ b/plugins/app/src/extensions/AppNav.tsx @@ -41,6 +41,7 @@ class NavItemBag implements NavContentNavItems { readonly #items: NavContentNavItem[]; readonly #index: Map; readonly #taken: Set; + #restItems: NavContentNavItem[] | undefined; constructor(items: NavContentNavItem[], taken?: Iterable) { this.#items = items; @@ -50,14 +51,27 @@ class NavItemBag implements NavContentNavItems { take(id: string): NavContentNavItem | undefined { const item = this.#index.get(id); - if (item) { + if (item && !this.#taken.has(id)) { this.#taken.add(id); + if (this.#restItems) { + const index = this.#restItems.findIndex( + restItem => restItem.node.spec.id === id, + ); + if (index !== -1) { + this.#restItems.splice(index, 1); + } + } } return item; } rest(): NavContentNavItem[] { - return this.#items.filter(item => !this.#taken.has(item.node.spec.id)); + if (!this.#restItems) { + this.#restItems = this.#items.filter( + item => !this.#taken.has(item.node.spec.id), + ); + } + return this.#restItems; } clone(): NavContentNavItems { @@ -65,19 +79,32 @@ class NavItemBag implements NavContentNavItems { } withComponent(Component: (props: NavContentNavItem) => JSX.Element) { + let renderedItems: JSX.Element[] | undefined; + return { take: (id: string) => { const item = this.take(id); - return item ? : null; + if (item && renderedItems) { + const index = renderedItems.findIndex( + renderedItem => renderedItem.key === item.node.spec.id, + ); + if (index !== -1) { + renderedItems.splice(index, 1); + } + } + return item ? : null; }, rest: (options?: { sortBy?: 'title' }) => { const items = this.rest(); - if (options?.sortBy === 'title') { - items.sort((a, b) => a.title.localeCompare(b.title)); + if (!renderedItems) { + if (options?.sortBy === 'title') { + items.sort((a, b) => a.title.localeCompare(b.title)); + } + renderedItems = items.map(item => ( + + )); } - return items.map(item => ( - - )); + return renderedItems; }, }; }