From 5f3f5d298bea465d644f94405deffb6939451ed7 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 17 Mar 2026 16:20:22 +0100 Subject: [PATCH] app-react: keep nav rest results in sync Make nav rest results stay live when additional items are taken later in the same render, which lets app nav layouts place specific items after collecting the remaining sidebar entries. Signed-off-by: Patrik Oldsberg Made-with: Cursor --- .changeset/live-nav-rest-items.md | 6 + packages/app/src/modules/appModuleNav.tsx | 14 +-- .../blueprints/NavContentBlueprint.test.tsx | 113 ++++++++++++++++-- .../src/blueprints/NavContentBlueprint.ts | 14 ++- plugins/app/src/extensions/AppNav.tsx | 43 +++++-- 5 files changed, 162 insertions(+), 28 deletions(-) create mode 100644 .changeset/live-nav-rest-items.md 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; }, }; }