yonlu/yellowjacket
1
1
Fork 0
Code Issues 55 Pull Requests 6 Actions Packages Projects Releases 12 Wiki Activity

Android: the bottom-nav item stops being highlighted after backing out of a detail view #72

New Issue
Closed
opened 2026-08-18 06:00:08 +00:00 by logan · 1 comment
logan commented 2026-08-18 06:00:08 +00:00
Collaborator
Copy Link
Copy Source

Report

Go to Albums → open an album → press the system back button to return to the albums grid: the Albums item in the bottom nav is no longer highlighted. It should stay highlighted until I go to another tab.

Findings

  • Back is history.back() replayed as popstate with _isBack (frontend/index.ts), and the nav components do not hear a navigate they did not send — this is the same class of bug as app-sidebar's default activeView having to be home to match the landing view.
  • So bottom-nav's active state is being set on outbound navigation but not restored on the back path.
  • A detail view is not a tab, so opening one presumably clears the highlight too — worth deciding whether the parent tab should stay lit while the detail is open (it should).

Direction

Have the shell publish the active primary view (it already publishes data-active-view on #main-content) and let bottom-nav and app-sidebar derive their highlight from that single fact, on every navigation including popstate. One source, no per-component tracking.

**Report** Go to Albums → open an album → press the system back button to return to the albums grid: the Albums item in the bottom nav is no longer highlighted. It should stay highlighted until I go to another tab. **Findings** - Back is `history.back()` replayed as `popstate` with `_isBack` (`frontend/index.ts`), and the nav components do not hear a `navigate` they did not send — this is the same class of bug as `app-sidebar`'s default `activeView` having to be `home` to match the landing view. - So `bottom-nav`'s active state is being set on outbound navigation but not restored on the back path. - A detail view is not a tab, so opening one presumably clears the highlight too — worth deciding whether the parent tab should stay lit *while* the detail is open (it should). **Direction** Have the shell publish the active *primary* view (it already publishes `data-active-view` on `#main-content`) and let `bottom-nav` and `app-sidebar` derive their highlight from that single fact, on every navigation including `popstate`. One source, no per-component tracking.
logan added the Area/Shell-NavKind/BugPlatform/Android
Priority
High
2
 labels 2026-08-18 14:36:17 +00:00
logan self-assigned this 2026-08-19 20:24:20 +00:00
logan added the
Status
In Progress
 label 2026-08-19 20:24:20 +00:00
logan commented 2026-08-19 20:24:21 +00:00
Author
Collaborator
Copy Link
Copy Source

Taking this on branch fix/72-active-view-broadcast.

Approach, following the Direction: the shell already knows the
active view — handleNavigate() sets #main-content's
data-active-view on every path including _isBack — and the nav
components learn it from the navigate CustomEvent, which only the
outbound path dispatches. So this is a missing broadcast in the shell,
not a bug in the two components. index.ts:191 is itself a document
listener for navigate, so re-dispatching that event from inside
handleNavigate is an infinite loop: the signal has to mean "the
active view is now X" rather than "please navigate to X".

One source of truth for the active primary view, published on every
navigation including popstate; both app-sidebar and bottom-nav
derive their highlight from it and stop tracking their own.

Two things measured beyond the report, at 1280px on
make dev-headless SEED=default:

  • It is not Android-only. Albums → Tracks → back lands on Albums
    with both navs highlighting Tracks — the view you just left. A
    confidently wrong highlight is worse than an absent one, and it is
    the more common case.
  • The two symptoms are one cause. app-sidebar.onGlobalNavigate
    guards on navItems.some(i => i.id === view), so a detail view leaves
    its highlight alone; bottom-nav has no such guard, so
    explore-album-details sets activeView to a value matching no tab
    and nothing lights. Neither is correct — the sidebar is stale-but-lucky
    — and per the report's third finding the parent tab should stay lit
    while a detail view is open, deliberately, in both.

e2e/specs/back-navigation.spec.ts already covers these journeys and
asserts only data-active-view, which is the one thing that is correct
on the back path — so it is green on the broken build. The assertions
go there rather than in a new file, on aria-current="page" via
getByRole.

Taking this on branch `fix/72-active-view-broadcast`. **Approach**, following the Direction: the shell already knows the active view — `handleNavigate()` sets `#main-content`'s `data-active-view` on every path *including* `_isBack` — and the nav components learn it from the `navigate` CustomEvent, which only the outbound path dispatches. So this is a missing broadcast in the shell, not a bug in the two components. `index.ts:191` is itself a `document` listener for `navigate`, so re-dispatching that event from inside `handleNavigate` is an infinite loop: the signal has to mean *"the active view is now X"* rather than *"please navigate to X"*. One source of truth for the active primary view, published on every navigation including `popstate`; both `app-sidebar` and `bottom-nav` derive their highlight from it and stop tracking their own. Two things measured beyond the report, at 1280px on `make dev-headless SEED=default`: - **It is not Android-only.** Albums → Tracks → back lands on Albums with *both* navs highlighting **Tracks** — the view you just left. A confidently wrong highlight is worse than an absent one, and it is the more common case. - **The two symptoms are one cause.** `app-sidebar.onGlobalNavigate` guards on `navItems.some(i => i.id === view)`, so a detail view leaves its highlight alone; `bottom-nav` has no such guard, so `explore-album-details` sets `activeView` to a value matching no tab and nothing lights. Neither is correct — the sidebar is stale-but-lucky — and per the report's third finding the parent tab should stay lit while a detail view is open, deliberately, in both. `e2e/specs/back-navigation.spec.ts` already covers these journeys and asserts only `data-active-view`, which is the one thing that is correct on the back path — so it is green on the broken build. The assertions go there rather than in a new file, on `aria-current="page"` via `getByRole`.
logan closed this issue 2026-08-19 21:10:55 +00:00
gitea-actions bot removed the
Status
In Progress
 label 2026-08-19 21:11:02 +00:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
Area/Design
Area/Downloads
Area/Explore
Area/Library-UI
Area/Metadata
Area/Packaging
Area/Player
Area/Queue
Area/Settings
Area/Shell-Nav
Compat/Breaking
Breaking change that won't be backward compatible
 Kind/Bug
Something is not working
 Kind/Documentation
Documentation changes
 Kind/Enhancement
Improve existing functionality
 Kind/Feature
New functionality
 Kind/Security
This is security issue
 Kind/Testing
Issue or pull request related to testing
 Platform/Android
Platform/Desktop
Priority
Critical
1
The priority is critical
Priority
High
2
The priority is high
Priority
Medium
3
The priority is medium
Priority
Low
4
The priority is low
Reviewed
Confirmed
1
Issue has been confirmed
Reviewed
Duplicate
2
This issue or pull request already exists
Reviewed
Invalid
3
Invalid issue
Reviewed
Won't Fix
3
This issue won't be fixed
Status
Blocked
1
Something is blocking this issue or pull request
Status
Need More Info
2
Feedback is required to reproduce issue or to continue work
Status
Abandoned
3
Somebody has started to work on this but abandoned work
Status
In Progress
Somebody is actively working on this right now
No labels Area/Shell-Nav Kind/Bug Platform/Android
Priority
High
2
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
logan yonlu
No Assignees logan
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: yonlu/yellowjacket#72