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

Android: the "More" navigation should not be a side drawer #71

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

Report

The "More" button in the bottom nav opens the sidebar as a drawer sliding in from the side. It looks wrong, and only part of the screen scrolls under my finger. It needs a more native-feeling mobile presentation.

Findings

  • bottom-nav's "More" renders the same <app-sidebar> inside a wa-drawer, deliberately — so the destinations are not listed twice. It is rendered only while the drawer is open, because an unconditional copy put a second data-testid="nav-*" in the DOM and broke 30 specs with strict-mode violations. Any replacement must keep that constraint.
  • The partial-scroll problem is the sidebar's own scroll container inside the drawer, plus missing overscroll-behavior.
  • The tab bar is deliberately four destinations plus a way to the rest; "More" must reach everything, Settings included.

Direction

A bottom sheet rising from the "More" button (matching the context-menu sheet, so there is one sheet pattern), full-width, with large rows and its own scroll container. Reuse the sidebar's item list as data rather than mounting <app-sidebar> itself if that is what it takes to get a native layout — but then the duplicate-testid rule needs re-checking. Sequence with the sidebar-toggles issue, which makes that list configurable.

**Report** The "More" button in the bottom nav opens the sidebar as a drawer sliding in from the side. It looks wrong, and only part of the screen scrolls under my finger. It needs a more native-feeling mobile presentation. **Findings** - `bottom-nav`'s "More" renders the *same* `<app-sidebar>` inside a `wa-drawer`, deliberately — so the destinations are not listed twice. It is rendered only while the drawer is open, because an unconditional copy put a second `data-testid="nav-*"` in the DOM and broke 30 specs with strict-mode violations. Any replacement must keep that constraint. - The partial-scroll problem is the sidebar's own scroll container inside the drawer, plus missing `overscroll-behavior`. - The tab bar is deliberately four destinations plus a way to the rest; "More" must reach everything, Settings included. **Direction** A bottom sheet rising from the "More" button (matching the context-menu sheet, so there is one sheet pattern), full-width, with large rows and its own scroll container. Reuse the sidebar's item list as data rather than mounting `<app-sidebar>` itself if that is what it takes to get a native layout — but then the duplicate-testid rule needs re-checking. Sequence with the sidebar-toggles issue, which makes that list configurable.
logan removed the
Status
Blocked
1
 label 2026-08-21 07:26:44 +00:00
logan commented 2026-08-21 07:26:57 +00:00
Author
Collaborator
Copy Link
Copy Source

Unblocked: #60 merged in #176, so the sole dependency is closed and
Status/Blocked is off.

What landed that you inherit rather than have to build:
components/menu-surface/ is the one decision about where a menu is
drawn — a wa-popup above 600px, a wa-dialog bottom sheet below it —
and all fourteen menu call sites go through it, including the playlist
submenu, which stacks as a second sheet. contextMenuStyles carries
the 48px rows for all of them, so a new menu gets thumb-sized targets
by using the same .context-menu-panel markup and nothing else.

Two things worth knowing before you add a menu:

  • A bare <wa-popup> will fail the build, deliberately.
    frontend/test/components/menu-surface.test.ts sweeps the source and
    allows three files, each with a stated reason. A popup is clipped on
    the reference device by .main-panel's paint containment and no
    tier here can see that    , so the guard is a source sweep rather than a
    rendered assertion.
  • The device measurements are in .planning/NOTES.md, dated
    2026-08-21 — including the two that would otherwise cost a rebuild: a
    top-layer <dialog> really does escape paint containment from inside
    a view, and a native dialog's UA max-width is what makes a naive
    sheet come out 354px wide on a 424px screen.
**Unblocked**: #60 merged in #176, so the sole dependency is closed and `Status/Blocked` is off. What landed that you inherit rather than have to build: `components/menu-surface/` is the one decision about *where* a menu is drawn — a `wa-popup` above 600px, a `wa-dialog` bottom sheet below it — and all fourteen menu call sites go through it, including the playlist submenu, which stacks as a second sheet. `contextMenuStyles` carries the 48px rows for all of them, so a new menu gets thumb-sized targets by using the same `.context-menu-panel` markup and nothing else. Two things worth knowing before you add a menu: - **A bare `<wa-popup>` will fail the build**, deliberately. `frontend/test/components/menu-surface.test.ts` sweeps the source and allows three files, each with a stated reason. A popup is clipped on the reference device by `.main-panel`'s paint containment and *no tier here can see that*, so the guard is a source sweep rather than a rendered assertion. - **The device measurements are in `.planning/NOTES.md`**, dated 2026-08-21 — including the two that would otherwise cost a rebuild: a top-layer `<dialog>` really does escape paint containment from inside a view, and a native dialog's UA `max-width` is what makes a naive sheet come out 354px wide on a 424px screen.
logan self-assigned this 2026-08-23 11:35:13 +00:00
logan added the
Status
In Progress
 label 2026-08-23 11:35:13 +00:00
logan commented 2026-08-23 11:35:13 +00:00
Author
Collaborator
Copy Link
Copy Source

Picking this up on branch feat/71-more-as-a-bottom-sheet, from origin/main.

Approach, following the Direction and #60's precedent:

  • The "More" surface becomes a bottom sheet — wa-drawer with
    placement="bottom", without-header and a grip, so it matches the
    context sheet menu-surface draws rather than being a second sheet
    pattern. wa-drawer is already a native <dialog> opened with
    showModal(), so the top-layer argument #60 measured holds here
    unchanged and nothing new has to be proved about paint containment.
  • The <app-sidebar> stays, mounted only while the sheet is open,
    because the duplicate-nav-*-testid rule the Findings name is still
    live: the shell's own sidebar is display: none below 600px, not
    removed. That also keeps #25's visibility toggles, the active-view
    highlight and the labels with one owner.
  • One scroll container. The report's "only part of the screen
    scrolls" is three nested scrollers — the dialog, its body, and the
    sidebar's own overflow-y: auto host. The sheet's body is the only
    one, with overscroll-behavior: contain.
  • Large rows: 48px, from #186's floor and #60's sheet rows,
    stated in app-sidebar's own stylesheet under the phone query, since
    that is the only place it renders below 600px.

Verification: make ui-test, make e2e against make dev-headless,
and a screenshot at 424x439 — this is a layout change, and this repo's
history says several of those were invisible to every assertion.

Picking this up on branch `feat/71-more-as-a-bottom-sheet`, from `origin/main`. **Approach**, following the Direction and #60's precedent: - The "More" surface becomes a **bottom sheet** — `wa-drawer` with `placement="bottom"`, `without-header` and a grip, so it matches the context sheet `menu-surface` draws rather than being a second sheet pattern. `wa-drawer` is already a native `<dialog>` opened with `showModal()`, so the top-layer argument #60 measured holds here unchanged and nothing new has to be proved about paint containment. - **The `<app-sidebar>` stays**, mounted only while the sheet is open, because the duplicate-`nav-*`-testid rule the Findings name is still live: the shell's own sidebar is `display: none` below 600px, not removed. That also keeps `#25`'s visibility toggles, the active-view highlight and the labels with one owner. - **One scroll container.** The report's "only part of the screen scrolls" is three nested scrollers — the dialog, its body, and the sidebar's own `overflow-y: auto` host. The sheet's body is the only one, with `overscroll-behavior: contain`. - **Large rows**: 48px, from `#186`'s floor and `#60`'s sheet rows, stated in `app-sidebar`'s own stylesheet under the phone query, since that is the only place it renders below 600px. Verification: `make ui-test`, `make e2e` against `make dev-headless`, and a screenshot at 424x439 — this is a layout change, and this repo's history says several of those were invisible to every assertion.
logan commented 2026-08-23 12:04:36 +00:00
Author
Collaborator
Copy Link
Copy Source

PR #211 — #211 — CI
green on both jobs, WebKit included.

The sheet is the same wa-drawer with placement="bottom" and
without-header, so it is #60's surface rather than a second sheet
pattern, and the sidebar is still mounted rather than re-listed as
data — the duplicate-nav-*-testid constraint the Findings name is
still live, since the shell's own copy is display: none below 600px
rather than removed. Measured at 424x439: 424 wide, 373 tall (85vh, so
there is an outside to tap), 48px rows, one scroller with
overscroll-behavior: contain, Settings reachable at the end of it.

Two things it deliberately does not do, both stated on the PR:

  • No scroll affordance yet. 412 of content in a 373px body means
    most of a row is below the fold with nothing saying so — #207's
    answer one surface over, filed here as #210, because #207 is an
    open PR and a second copy of an unshipped rule is how the two sheets
    come to disagree about what a fold looks like.
  • The back gesture still does not dismiss it, which was true of the
    side drawer too and is #200's subject.

Leaving Status/In Progress on until the PR merges.

**PR #211** — https://git.ljones.me/yonlu/yellowjacket/pulls/211 — CI green on both jobs, WebKit included. The sheet is the *same* `wa-drawer` with `placement="bottom"` and `without-header`, so it is #60's surface rather than a second sheet pattern, and the sidebar is still mounted rather than re-listed as data — the duplicate-`nav-*`-testid constraint the Findings name is still live, since the shell's own copy is `display: none` below 600px rather than removed. Measured at 424x439: 424 wide, 373 tall (85vh, so there is an outside to tap), 48px rows, one scroller with `overscroll-behavior: contain`, Settings reachable at the end of it. Two things it deliberately does not do, both stated on the PR: - **No scroll affordance yet.** 412 of content in a 373px body means most of a row is below the fold with nothing saying so — #207's answer one surface over, filed here as **#210**, because #207 is an open PR and a second copy of an unshipped rule is how the two sheets come to disagree about what a fold looks like. - **The back gesture still does not dismiss it**, which was true of the side drawer too and is #200's subject. Leaving `Status/In Progress` on until the PR merges.
logan closed this issue 2026-08-25 17:55:31 +00:00
gitea-actions bot removed the
Status
In Progress
 label 2026-08-25 18:07:28 +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/Enhancement Platform/Android
Priority
Medium
3
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.
Depends on
#60 Android: context menus should be a bottom sheet, not a popover that gets clipped
yonlu/yellowjacket
Reference: yonlu/yellowjacket#71