From f18691560d9475b5dbb0bd89e88baffc76155d68 Mon Sep 17 00:00:00 2001 From: Logan Date: Wed, 19 Aug 2026 16:45:47 -0400 Subject: [PATCH] fix(shell): publish the active view, so both navs follow the back path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The nav components learned where the user was from the `navigate` CustomEvent, which only the outbound path dispatches: `popstate` calls `handleNavigate()` directly. So a back-navigation left both of them highlighting the view just left — desktop included, at any width, on any back across two primary views. Opening a detail view was the same cause wearing a different symptom: `app-sidebar` guarded on its own item list and kept its highlight, `bottom-nav` did not and lit nothing. It cannot be fixed by re-dispatching `navigate` — `index.ts` is that event's document listener, so that is an infinite loop, and "please go to X" is not the statement being made. `activeViewStore` is the shell saying "the active view is now X", once per navigation, `popstate` included; both navs read it through a controller and hold no `activeView` of their own. A store rather than an event because a component that mounts *after* a navigation still has to know: `bottom-nav`'s drawer builds its `app-sidebar` on open, and that copy had heard nothing at all, so the drawer opened on Home from any page in the app. Closes #72 --- frontend/index.ts | 15 ++++ .../src/components/bottom-nav/bottom-nav.ts | 29 ++++-- .../src/components/sidebar/app-sidebar.ts | 52 +++++------ frontend/src/store/active-view-store.ts | 90 +++++++++++++++++++ .../controllers/active-view-controller.ts | 59 ++++++++++++ frontend/src/store/index.ts | 2 + frontend/test/components/bottom-nav.test.ts | 77 +++++++++++----- frontend/test/components/chrome.test.ts | 37 +++++++- frontend/test/stores/view-stores.test.ts | 59 +++++++++++- 9 files changed, 355 insertions(+), 65 deletions(-) create mode 100644 frontend/src/store/active-view-store.ts create mode 100644 frontend/src/store/controllers/active-view-controller.ts diff --git a/frontend/index.ts b/frontend/index.ts index fee2280..cb7396a 100644 --- a/frontend/index.ts +++ b/frontend/index.ts @@ -40,6 +40,7 @@ import { setBasePath } from '@awesome.me/webawesome/dist/webawesome.js'; import { registerBundledIcons } from './src/icons'; import { queueStore } from '@store/queue-store'; import { searchStore } from '@store/search-store'; +import { activeViewStore } from '@store/active-view-store'; import * as Player from '@go/player/player.js'; import * as Queue from '@go/queue/queue.js'; import { GetDefaultPage } from '@go/config/config.js'; @@ -279,6 +280,20 @@ async function handleNavigate( // attribute keeps e2e selectors semantic instead of structural. mainContent.dataset.activeView = view; + // And publishing it as a *value* is what the nav components read. + // They used to learn the active view from the `navigate` event, + // which only the outbound path dispatches -- so a back-navigation + // left both of them highlighting the view it had just left (#72). + // Re-dispatching `navigate` here is not the fix: this file is a + // document listener for it, so that is an infinite loop, and + // "please go to X" is not the statement being made. + // + // `view in VIEW_TAGS` is the primary/detail split, and it is passed + // rather than re-derived because this table is where it is written + // down. A detail view therefore leaves the tab it was opened from + // lit, which is what the report asks for. + activeViewStore.setView(view, view in VIEW_TAGS); + // --- Primary (cacheable) views ---------------------------------------- if (view in VIEW_TAGS) { // Remove any active detail view first diff --git a/frontend/src/components/bottom-nav/bottom-nav.ts b/frontend/src/components/bottom-nav/bottom-nav.ts index 4cae8f2..24f6d7c 100644 --- a/frontend/src/components/bottom-nav/bottom-nav.ts +++ b/frontend/src/components/bottom-nav/bottom-nav.ts @@ -7,6 +7,7 @@ import { designTokens } from '../../styles/tokens.css'; import '../sidebar/app-sidebar.js'; import { nameDialog } from '@utils/name-dialog'; import { ICON_PLAYLIST } from '@utils/icon-language'; +import { ActiveViewController } from '@store/controllers/active-view-controller'; type View = 'home' | 'albums' | 'tracks' | 'playlists'; @@ -114,8 +115,19 @@ export class BottomNav extends LitElement { } `]; - @state() - private activeView = 'home'; + /** + * Which tab is lit, read from the shell rather than tracked here. + * + * This was a `@state()` field set from the `navigate` event, which + * only the outbound path dispatches -- so backing out of a detail + * view left the highlight wherever it had been (#72). It had no + * equivalent of `app-sidebar`'s `navItems.some(...)` guard either, + * so a detail view set it to a name matching no tab and *nothing* + * was lit; that asymmetry is why one nav looked broken and the + * other looked fine. The store answers both: a detail view leaves + * the tab it was opened from lit, in both components. + */ + private activeCtrl = new ActiveViewController(this); /** * Whether the drawer has been asked for. @@ -167,12 +179,9 @@ export class BottomNav extends LitElement { nameDialog(this.drawer); } - private onGlobalNavigate = (e: Event) => { - const detail = (e as CustomEvent<{ view?: string }>).detail; - - if (detail?.view) this.activeView = detail.view; - + private onGlobalNavigate = () => { // A navigation from inside the drawer is the drawer's job done. + // The highlight is not this listener's business any more. this.drawerOpen = false; }; @@ -206,9 +215,11 @@ export class BottomNav extends LitElement {