Compare commits
26
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
f967916550 | ||
|
|
3fa7c7734b | ||
|
|
cceeb40b16 | ||
|
|
2926ecd4b4 | ||
|
|
ff3c4003cb | ||
|
|
def596a99e | ||
|
|
14f78c0b57 | ||
|
|
7cea238e71 | ||
|
|
4f2f1827ab | ||
|
|
e454e4074b | ||
|
|
c518ac8c73 | ||
|
|
977f624123 | ||
|
|
23f3d4b3b0 | ||
|
|
8d46c4abb7 | ||
|
|
f714fe513d | ||
|
|
087c69ac8d | ||
|
|
bb7dde1963 | ||
|
|
446380e3a9 | ||
|
|
e07f248cc8 | ||
|
|
90ac6e0825 | ||
|
|
4e3c953acf | ||
|
|
ede183d026 | ||
|
|
481c9dca65 | ||
|
|
4025106234 | ||
|
|
a3134f997f | ||
|
|
3607fe445e |
@@ -3580,3 +3580,119 @@ per-card flag can answer at all.
|
||||
The general point: **two columns that agree today are not one column.**
|
||||
Which of them a new surface reads should be decided by which one has
|
||||
something that can un-set it.
|
||||
|
||||
## The queue panel was a column that could not afford to be one (measured 2026-08-19)
|
||||
|
||||
Plan 018, issue #24. Measured against the running app (`make
|
||||
dev-headless SEED=default`, Chromium) on Playlists, sweeping the
|
||||
viewport with the queue open and closed. Main panel width, and how much
|
||||
of the page header survived:
|
||||
|
||||
| viewport | sidebar | main (queue open) | actions clipped |
|
||||
|---|---|---|---|
|
||||
| 1280×800 | 200 | 759 | — |
|
||||
| 1000×700 | 200 | 479 | 2 of 3 |
|
||||
| **900×600** | 200 | **379** | all three |
|
||||
| 800×600 | 56 | 423 | all three |
|
||||
| 390×780 | — | **69** | all three |
|
||||
| 320×600 | — | **0** | all three |
|
||||
| 800×600 | 56 | 744 *(closed)* | New Smart Playlist, 158/162px |
|
||||
|
||||
Five things came out of it that the issue did not say.
|
||||
|
||||
- **The header clips at the enforced minimum with the queue closed.**
|
||||
800×600 is the only size this app promises, and "New Smart Playlist"
|
||||
loses 4px of its 162 there. The queue makes it dramatic; it is not
|
||||
the cause.
|
||||
- **900×600 is worse than 800×600.** `AUTO_COLLAPSE_VIEWPORT` collapses
|
||||
the sidebar *below* 900, so the main panel is 843px at 899 and 700px
|
||||
at 900. **The worst desktop case is the top of the Compact band, not
|
||||
the enforced floor** — so every viewport list that stopped at "the
|
||||
minimum" was missing its own worst case. `layout-overflow.spec.ts`
|
||||
carries 900 now.
|
||||
- **At 320px the main panel was 0px.** The panel is `flex-shrink: 0` in
|
||||
the flow of `.content-area`, so an open queue is paid for by the
|
||||
content rather than covering it. Not degraded — gone. That is the
|
||||
measurement #55 wanted and did not have.
|
||||
- **Only Playlists overflows.** All ten primary views swept at 900×600
|
||||
and 390×780; every other header reports `scrollWidth ==
|
||||
clientWidth`, and Albums at 390 renders title, count and sort legibly
|
||||
(checked on a screenshot, not just the number). So #69 is one view's
|
||||
action set — three text buttons totalling 390px — and not a systemic
|
||||
header failure.
|
||||
- **Both reasons in `MinWidth`'s comment had expired.** The subtitle is
|
||||
`display: none` from 899 down, and the sidebar host is
|
||||
`overflow-y: auto` (at 600×460, `scrollHeight` 434 against a 332px
|
||||
client, Settings reachable after scrolling). The floor is right; its
|
||||
stated defence was two mechanisms that can no longer happen, which is
|
||||
worse than either answer because nobody can argue with it.
|
||||
|
||||
**A correction worth keeping, because it nearly went in the plan.** My
|
||||
first probe for the sidebar's scroller searched
|
||||
`shadowRoot.querySelectorAll('*')` and reported "no scroller — items
|
||||
are unreachable", which reads exactly like a live Settings-unreachable
|
||||
bug. The scroller is the **host**, and a host is not inside its own
|
||||
shadow root. CLAUDE.md was right and the probe was wrong.
|
||||
|
||||
**And one claim in the plan's first draft was too strong**: that the
|
||||
overlay "removes the desktop half of #69". After phase 2, at 900×600,
|
||||
open and closed are now *identical* (main 700, one action clipped)
|
||||
where open used to be main 379 with all three clipped. The queue's
|
||||
contribution is gone; the header's own overflow remains and is still a
|
||||
live defect at a supported size.
|
||||
|
||||
### The mode cannot be a media query
|
||||
|
||||
The panel is drag-resizable 200–500px and persisted, so a viewport
|
||||
breakpoint assumes the default 320 and is wrong by up to 180px for a
|
||||
user who widened it — in the direction that hurts, since a wider queue
|
||||
is exactly when the content can least afford it. It is computed from
|
||||
`.content-area`'s width instead (which already accounts for the
|
||||
sidebar's collapse), and the component test that matters widens the
|
||||
panel at a *fixed* parent width and asserts the flip.
|
||||
|
||||
The floor (480) is a judgement, and the measurement is why: there is no
|
||||
cliff. The track list rescales its columns continuously — 213px down to
|
||||
124px between main widths of 900 and 544, `rowOverflow=0` at every step
|
||||
— and the album grid steps 3 columns to 2 somewhere between 564 and 644
|
||||
without breaking. So 480 is anchored at both ends instead: it keeps the
|
||||
default 1100px window inline, and puts every measured-broken case on
|
||||
the overlay side.
|
||||
|
||||
The scrim is perceptible but subtle on a dark ramp, which is worth
|
||||
knowing before someone "fixes" it as broken: sampled from screenshots at
|
||||
900×600, the main panel's background goes 33,37,41 → 18,20,23 and a
|
||||
row's text 242 → 133. It covers the content area only — not the sidebar
|
||||
or the transport — because the queue is not modal.
|
||||
|
||||
## No test tier can see a `hover:` media query (measured 2026-08-19)
|
||||
|
||||
Gating an affordance on `(hover: hover) and (pointer: fine)` — #68's fix
|
||||
for the play button that flashed on a long-press — is invisible to both
|
||||
browser tiers, in *different* ways, and neither of them fails.
|
||||
|
||||
- **`make ui-test`**: CDP's `Emulation.setEmulatedMedia` with a `hover`
|
||||
feature does not reach the tier's iframe. The call succeeds and
|
||||
`matchMedia('(hover: hover)')` still answers `true` afterwards. So
|
||||
there is no way to render a component as a phone would and read the
|
||||
computed style.
|
||||
- **`make e2e`**: both projects are desktop (`Desktop Chrome`,
|
||||
`Desktop Safari`), and the phone specs reach phone *width* with
|
||||
`setViewportSize`, which changes no media feature but `width`. So the
|
||||
phone specs run with `hover: hover` and the gate is never exercised.
|
||||
|
||||
What does work, and what the fix was verified with, is a second browser
|
||||
context under a device descriptor: `chromium.newContext(devices['Pixel
|
||||
5'])` reports `hover=false pointer:fine=false` and the button computes
|
||||
`display: none`, against `flex` at 1440px. That is a one-off script, not
|
||||
a spec — `isMobile` is Chromium-only, so it cannot become an e2e project
|
||||
without losing the WebKit half.
|
||||
|
||||
`hover-affordance.test.ts` therefore asserts the *parsed stylesheet* —
|
||||
that the reveal rule sits inside the media query — which catches the
|
||||
regression that actually threatens it: someone hoisting the rule back out
|
||||
as a tidy-up, a change nothing on a desktop renders differently.
|
||||
|
||||
Related: a width-gated decision **is** testable at both tiers, which is
|
||||
why #61's phone mini player is a `matchMedia` stub in the component test
|
||||
and needs nothing special.
|
||||
|
||||
@@ -0,0 +1,327 @@
|
||||
# 018 — Supported sizes, and what the queue panel is
|
||||
|
||||
**Issue:** #24 (`Area/Shell-Nav`, `Priority/High`, `Reviewed/Confirmed`)
|
||||
**Unblocks:** #55 (queue as a screen) — a real Gitea dependency
|
||||
**Relates:** #69 (page-header overflow), #12 (mini-player), #51 (small-screen umbrella)
|
||||
**Status:** complete — #24 shipped as PR #132, and the matrix's last
|
||||
unkept promise closed with #69.
|
||||
|
||||
#73 puts this first in Phase 2 and hangs the rest of the phase off it,
|
||||
so the decision has to be written down and arguable before any CSS
|
||||
moves. This document is the decision. Everything below the matrix is
|
||||
either a measurement or an argument for one of the four choices #24
|
||||
asks for.
|
||||
|
||||
---
|
||||
|
||||
## What is actually wrong, measured
|
||||
|
||||
Against the running app (`make dev-headless SEED=default`, Chromium),
|
||||
Playlists, sweeping the viewport with the queue open and closed. The
|
||||
number that matters is how much of the page header survives.
|
||||
|
||||
| viewport | sidebar | queue | main panel | header needs | actions clipped |
|
||||
|---|---|---|---|---|---|
|
||||
| 1280×800 | 200 | open 321 | 759 | 759 | — |
|
||||
| 1000×700 | 200 | open 321 | 479 | 747 | New Playlist, New Smart Playlist |
|
||||
| **900×600** | 200 | open 321 | **379** | 747 | **all three** |
|
||||
| 800×600 | 56 | open 321 | 423 | 747 | all three |
|
||||
| 700×600 | 56 | open 321 | 323 | 747 | all three |
|
||||
| 390×780 | — | open 321 | **69** | 747 | all three |
|
||||
| 320×600 | — | open 321 | **0** | 747 | all three |
|
||||
| 900×600 | 200 | closed | 700 | 747 | New Smart Playlist |
|
||||
| **800×600** | 56 | closed | 744 | 747 | **New Smart Playlist (158/162px)** |
|
||||
| 320×600 | — | closed | 320 | 747 | all three |
|
||||
|
||||
Five things in that table are not in the issue.
|
||||
|
||||
**The header clips at the supported minimum with the queue closed.**
|
||||
At 800×600 — the size `backend/config/window.go` enforces and the only
|
||||
size this app *promises* — "New Smart Playlist" loses 4px of its 162.
|
||||
#24 reads as a queue-panel bug; the queue makes it dramatic, but the
|
||||
header overflows on its own at the minimum window.
|
||||
|
||||
**900×600 is worse than 800×600, because the sidebar expands at 900.**
|
||||
`AUTO_COLLAPSE_VIEWPORT` collapses the sidebar to icons *below* 900, so
|
||||
at 899px the main panel is 843px and at 900px it is 700px. The worst
|
||||
desktop case is therefore not the minimum window; it is the pixel
|
||||
immediately above the collapse. Anything that tests "the minimum" and
|
||||
stops has not tested the worst case, which is what
|
||||
`layout-overflow.spec.ts` does today.
|
||||
|
||||
**At phone widths the queue is not a drawer, it is an amputation.**
|
||||
`queue-panel`'s host is `flex-shrink: 0; width: 0`, going to
|
||||
`width: var(--queue-width, 320px)` under `[open]` — it is *in the flow*
|
||||
of `.content-area`, so it takes its width from the main panel rather
|
||||
than covering it. At 390px that leaves 69px of the page; at 320px it
|
||||
leaves **0px**, and the app is not degraded but gone. This is the
|
||||
measurement #55 needs and did not have.
|
||||
|
||||
**Only Playlists overflows.** Sweeping all ten primary views at 900×600
|
||||
and at 390×780, every other header reports `scrollWidth ==
|
||||
clientWidth`, and Albums at 390px renders title, count and sort
|
||||
legibly (checked on a screenshot, not just the number). #69 is
|
||||
therefore one view's action set — three text buttons totalling 390px —
|
||||
and not a systemic header failure, though the *rule* still belongs in
|
||||
`page-header`.
|
||||
|
||||
**Both reasons in `MinWidth`'s comment are stale.** It says the floor is
|
||||
800×600 because "below ~780 the header's subtitle wraps" and "below
|
||||
~600 tall the eleven sidebar items no longer fit". The subtitle is
|
||||
`display: none` below 900 (index.css), and the sidebar host is
|
||||
`overflow-y: auto` — at 600×460 its `scrollHeight` is 434 against a
|
||||
332px client, and Settings is reachable after scrolling. Neither
|
||||
mechanism can happen any more. That does not mean the floor should
|
||||
move; it means its stated reason no longer supports it, which is worse
|
||||
than either answer.
|
||||
|
||||
*(Care needed: my first probe for the sidebar scroller searched
|
||||
`shadowRoot.querySelectorAll('*')` and reported "items are
|
||||
unreachable", because the scroller is the **host** and a host is not in
|
||||
its own shadow root. The claim in CLAUDE.md is correct.)*
|
||||
|
||||
---
|
||||
|
||||
## Decision 1 — the supported size matrix
|
||||
|
||||
Three bands. Two of them already exist and are already argued; what is
|
||||
new is that they are written down as a *promise*, and that the queue is
|
||||
part of it.
|
||||
|
||||
| band | width | navigation | queue | promise |
|
||||
|---|---|---|---|---|
|
||||
| **Phone** | < 600 | `bottom-nav` + drawer | overlay, full width | reflows; nothing needs sideways scrolling; fits 320px |
|
||||
| **Compact** | 600 – 899 | icon sidebar | overlay + scrim | nothing is clipped or unreachable at any width in the band |
|
||||
| **Desktop** | ≥ 900 | labelled sidebar | inline where it fits (see decision 2), else overlay | as Compact |
|
||||
|
||||
And one promise across all three: **no action is ever unreachable.**
|
||||
That is the sentence #69 asks for and it is the one the matrix exists
|
||||
to make checkable.
|
||||
|
||||
**400% zoom** keeps the meaning it already has: WCAG 1.4.10 names 320px
|
||||
as the reflow target, the phone band covers it, and
|
||||
`layout-overflow.spec.ts` already asserts a 320px viewport needs no
|
||||
sideways scrolling. What changes is that the *queue* must be part of
|
||||
that assertion — it is not today, and with the queue open at 320px the
|
||||
main panel is 0px wide, which no current test can see.
|
||||
|
||||
**The window minimum stays 800×600**, and its comment gets the real
|
||||
reason. The old mechanisms are gone, but the floor is still where the
|
||||
Compact band's chrome stops being comfortable, and lowering it would
|
||||
mean promising the desktop layout at sizes where only the phone layout
|
||||
works. The interesting consequence is decision 4.
|
||||
|
||||
---
|
||||
|
||||
## Decision 2 — the queue is an overlay when it cannot afford to be a column
|
||||
|
||||
**The rule.** The queue panel renders inline — in the flow, as today —
|
||||
only while
|
||||
|
||||
```
|
||||
viewport − sidebar − queueWidth ≥ 480
|
||||
```
|
||||
|
||||
and as an overlay with a scrim otherwise.
|
||||
|
||||
**Why it cannot be a media query**, which is the load-bearing half:
|
||||
the queue's width is *user state*. It is drag-resizable between 200 and
|
||||
500px and persisted (`--queue-width`, `MIN_WIDTH`/`MAX_WIDTH` in
|
||||
`queue-panel.ts`). A breakpoint at a fixed viewport width silently
|
||||
assumes the default 320, and is wrong by 180px for a user who has
|
||||
dragged the panel wide — in the direction that hurts, since a wider
|
||||
queue is exactly when the content can least afford it. So the mode is
|
||||
computed from the measured widths and published as an attribute, the
|
||||
way `data-active-view` already is, and the CSS keys off that.
|
||||
|
||||
**Why 480, honestly.** There is no cliff to derive it from. The track
|
||||
list rescales its columns continuously — at main widths from 900 down
|
||||
to 544 its `--grid-cols` shrink from 213px to 124px with
|
||||
`rowOverflow=0` throughout — and the album grid steps 3 columns to 2
|
||||
somewhere between 564 and 644 without breaking. So this is a judgement,
|
||||
anchored on two things: it keeps the *default* window (1100 wide, main
|
||||
= 580) inline, because the inline queue is a desktop affordance people
|
||||
choose and turning it into an overlay for the common case would be a
|
||||
regression in feel; and it puts every case measured as broken —
|
||||
900×600 at main=379, and every phone width — on the overlay side.
|
||||
1024×768 lands at main=504 and stays inline.
|
||||
|
||||
**The scrim is the other half of the issue's complaint** ("make the
|
||||
queue obviously an overlay *over* the content so it reads as something
|
||||
to close"). An overlay queue gets a scrim, closes on scrim click and on
|
||||
Escape, and returns focus to `#queue-button`.
|
||||
|
||||
**What must not change**: #55's Direction is explicit — one component,
|
||||
two mount points, do not fork it. The overlay is a *presentation* of
|
||||
the same `queue-panel`, so the roving tab stop, Alt+Arrow reorder, drag
|
||||
reorder, selection semantics and the `virtualizer.requestUpdate()` on
|
||||
selection and current-track change all come along untouched. This
|
||||
decision deliberately stops short of #55's detail-view mount, but it is
|
||||
the shape that makes it possible, and it unblocks it.
|
||||
|
||||
---
|
||||
|
||||
## Decision 3 — #69 is its own PR, and here is the finding that decides it
|
||||
|
||||
`page-header` **cannot collapse its own actions**, and that is not an
|
||||
effort estimate but a fact about the API. Actions arrive through
|
||||
`<slot name="actions">` as arbitrary light-DOM markup — Playlists slots
|
||||
a `<div class="header-actions">` of three `<button>`s with click
|
||||
handlers, drag handlers and a conditional class. A component cannot
|
||||
move another component's light-DOM children into a dropdown and keep
|
||||
their behaviour; there is nothing generic to render as a menu item.
|
||||
|
||||
So the overflow rule needs an *actions API* — hosts declaring
|
||||
`{icon, label, handler, priority}` data that `page-header` can render
|
||||
either as buttons or as menu items — which is a change to all three
|
||||
hosts that slot actions, not a rule added in one place. That is a
|
||||
different piece of work from this one, it is independently verifiable,
|
||||
and the desktop half of #69's symptom is removed by decision 2 anyway
|
||||
(the queue stops eating the header's width).
|
||||
|
||||
It therefore stays #69, gets the finding above recorded on it, and
|
||||
follows immediately after this. What *this* plan owes it is the
|
||||
promise in the matrix — no action unreachable at any supported size —
|
||||
and the measurement that the only offender today is Playlists.
|
||||
|
||||
**And the promise is not kept yet, which is the honest version of a
|
||||
claim this document made in its first draft.** "Decision 2 removes the
|
||||
desktop half of #69's symptom" was too strong. Measured after phase 2,
|
||||
at 900×600 on Playlists:
|
||||
|
||||
| | before | after |
|
||||
|---|---|---|
|
||||
| queue open | main 379px, **all three** actions clipped | main 700px, **one** clipped |
|
||||
| queue closed | main 700px, one clipped | unchanged |
|
||||
|
||||
So the queue's *contribution* is gone — open and closed are now
|
||||
identical, which is the whole of what this decision owed — and the
|
||||
residual "New Smart Playlist: 114/162px" is the header overflowing on
|
||||
its own, at a size the queue never touched. #69 is still a live defect
|
||||
at a supported size, and the matrix's promise is what will close it.
|
||||
|
||||
---
|
||||
|
||||
## Decision 4 — a very small window becomes the phone layout, not the mini-player
|
||||
|
||||
#24 asks whether a very small window should switch to the mini-player
|
||||
(#12) "or simply refuse to go there". Both options in the question are
|
||||
worse than the one the codebase already has.
|
||||
|
||||
**#12 is a second window, not a mode.** Its findings say so: v3
|
||||
supports multiple windows, `AlwaysOnTop` is a window *option*, and the
|
||||
frontend would need an entry branch mounting only the mini-player root
|
||||
for a second window loading the same bundle. Turning the main window
|
||||
into a mini-player at some width conflates the two: it would throw away
|
||||
the user's navigation state on a resize, and it puts the MPRIS question
|
||||
(#12's own open question — media controls are process-level and must
|
||||
not be per-window) on a code path that a drag can trigger by accident.
|
||||
|
||||
**And "refuses" is unnecessary, because the reflow already exists.**
|
||||
The phone band is real, tested, and reached by width alone — a desktop
|
||||
window narrowed below 600px already gets `bottom-nav` and the phone
|
||||
shell. That is a better answer than refusing: it is strictly more
|
||||
usable than a hard minimum, it costs nothing new, and it is the same
|
||||
code Android runs, so it stays exercised.
|
||||
|
||||
So: the main window reflows and never becomes a mini-player; #12 stays
|
||||
a separate always-on-top window and is not blocked by, or coupled to,
|
||||
this decision. The window minimum stays 800×600 for the reason in
|
||||
decision 1 — but the phone band is what happens below it, not a
|
||||
refusal, which is why the minimum is a comfort floor rather than a
|
||||
correctness one.
|
||||
|
||||
---
|
||||
|
||||
## Phases
|
||||
|
||||
1. **This document**, linked from #24, with the matrix reported on the
|
||||
issue and #55 told whether it is unblocked. *(no code)* — **done**
|
||||
2. **The queue's overlay mode** — computed mode attribute, scrim,
|
||||
Escape and scrim-click close, focus return. The inline path is
|
||||
unchanged above the threshold. — **done**
|
||||
3. **The window minimum's comment** — replace both stale reasons with
|
||||
the measured ones. No value change. — **done**
|
||||
4. **Verification**, below. Including the specs that must change
|
||||
because they assert the old behaviour. — **done**
|
||||
|
||||
#69 follows as its own branch; #55 became unblocked at phase 2.
|
||||
|
||||
## What landed, measured
|
||||
|
||||
Main panel width with the queue open, before and after:
|
||||
|
||||
| viewport | before | after | mode |
|
||||
|---|---|---|---|
|
||||
| 1280×800 | 759 | 759 | inline |
|
||||
| 1100×720 (default window) | 579 | 579 | inline |
|
||||
| 1024×768 | 503 | 503 | inline |
|
||||
| 900×600 | **379** | **700** | overlay |
|
||||
| 800×600 | 423 | 744 | overlay |
|
||||
| 390×780 | **69** | **390** | overlay |
|
||||
| 320×600 | **0** | **320** | overlay |
|
||||
|
||||
The scrim is perceptible but subtle on a dark ramp, which is worth
|
||||
knowing before someone "fixes" it: sampled from the screenshots at
|
||||
900×600, the main panel's background goes 33,37,41 → 18,20,23 and a
|
||||
row's text 242 → 133. It covers the **content area only** — not the
|
||||
sidebar or the transport — on purpose: the queue is not modal, and
|
||||
leaving the navigation live means the scrim reads as "this is over the
|
||||
content" (which is what #24 asked for) without pretending the rest of
|
||||
the app is unavailable.
|
||||
|
||||
## What #69 did with the promise, and one thing this plan got wrong
|
||||
|
||||
#69 landed on its own branch as decision 3 said it would, and the
|
||||
matrix's *no action is ever unreachable at any supported size* is now
|
||||
kept rather than promised. Measured on Playlists, actions clipped:
|
||||
|
||||
| viewport | before #24 | after #24 | after #69 |
|
||||
|---|---|---|---|
|
||||
| 900×600, queue open | all three | one (114/162px) | none |
|
||||
| 900×600, queue closed | one | one | none |
|
||||
| 800×600, queue closed | one (158/162px) | one | none |
|
||||
| 390×780 | all three | all three | none |
|
||||
| 320×600 | all three | all three | none |
|
||||
|
||||
The shape was the one decision 3 predicted — an actions API first, an
|
||||
overflow rule second — and all three hosts that slot actions migrated.
|
||||
|
||||
**What this document got wrong is smaller and worth keeping.** Decision
|
||||
1 says the header's minimum is a *comfort* floor and that only the
|
||||
queue and the actions compete for the header's width. They are not the
|
||||
only two: every child of that flex row was `flex-shrink: 0`, so
|
||||
whatever came last lost, and the actions come last. At 320px the sort
|
||||
control alone is 172px of the header — so with every action already
|
||||
collapsed into the menu, the *menu button* was 76px off the right edge.
|
||||
The promise was still broken with nothing left to collapse.
|
||||
|
||||
That is why #69 also had to decide what gives way: the title (which the
|
||||
navigation also states) and, below 600px, the word "Sort:" (which the
|
||||
direction arrow implies). Neither is an action, which is the rule the
|
||||
matrix actually encodes — **an action is a capability and everything
|
||||
else on that row is a label.**
|
||||
|
||||
## Verification, and what each tier cannot see
|
||||
|
||||
- `make ui-test` — the queue panel's mode logic is component-tier
|
||||
work and belongs there. It **cannot** see the shell: the threshold is
|
||||
computed from the sidebar and viewport, which do not exist in that
|
||||
tier.
|
||||
- `make e2e` — `layout-overflow.spec.ts` gains the queue-open case at
|
||||
every band (it has none today, which is why main=0px at 320px has
|
||||
never failed anything) and **gains 900×600**, since the minimum is
|
||||
not the worst case. `queue-toggle-state.spec.ts` and
|
||||
`phone-shell.spec.ts` both touch the panel and must be re-read before
|
||||
editing.
|
||||
- **Screenshots at every band, read by a human.** This is not optional
|
||||
here: `layout-overflow.spec.ts` asserts the *shell* needs no sideways
|
||||
scrolling and passes on a build whose album header clips its own
|
||||
buttons (measured this session at 390px; filed on #66). Clipping
|
||||
*inside* a component is invisible to it, and clipping is this issue.
|
||||
- `make ui-visual` **cannot help at all** — the component tier renders
|
||||
the token fallbacks, because the theme only reaches `:root` in the
|
||||
real app.
|
||||
- Accessible names via `page.getByRole(...)`, never a shadow-root
|
||||
query. A drawer with a scrim is exactly the shape that grows a
|
||||
nameless control, and this repo has shipped one three times.
|
||||
@@ -1328,6 +1328,68 @@ is 32px each. Which four is plan 016's committed subset, and everything
|
||||
else — Settings included, because a phone still needs it — is behind
|
||||
"More".
|
||||
|
||||
**There are three supported size bands, and the queue is part of the
|
||||
promise.** Plan 018 (#24) wrote them down: **Phone** below 600 (bottom
|
||||
nav, reflows, fits 320px exactly), **Compact** 600–899 (icon sidebar),
|
||||
**Desktop** from 900 (labelled sidebar) — plus one sentence across all
|
||||
three, *no action is ever unreachable at any supported size*. The bands
|
||||
themselves already existed; what was new is that they are a promise and
|
||||
that the queue panel is inside it.
|
||||
|
||||
**900 is the worst desktop width, not the 800×600 minimum.** The
|
||||
sidebar collapses to icons *below* 900, so the main panel is 843px at
|
||||
899 and 700px at 900 — the narrowest content area any desktop width
|
||||
produces is at the top of the Compact band, not at the enforced floor.
|
||||
Every viewport list that stopped at "the minimum" was therefore missing
|
||||
its own worst case, which is why `layout-overflow.spec.ts` carries 900
|
||||
now. And **both reasons in `MinWidth`'s comment had expired** — the
|
||||
subtitle is `display: none` from 899 down and the sidebar host scrolls
|
||||
(`overflow-y: auto`; at 600×460 its `scrollHeight` is 434 against a
|
||||
332px client) — so 800×600 is a *comfort* floor for desktop chrome and
|
||||
not a correctness one. Below it the phone layout takes over, which is
|
||||
also why a very small window reflows rather than becoming a
|
||||
mini-player: **#12 is a second always-on-top window, not a mode of this
|
||||
one**, and making it a mode would discard navigation state on a resize
|
||||
and put the process-level MPRIS question on a path a drag can trigger.
|
||||
|
||||
**The queue panel is a column only while the content can spare the
|
||||
width, and that cannot be a media query.** In flow the host is
|
||||
`flex-shrink: 0`, so an open queue is paid for by the main panel: it
|
||||
left 379px at 900×600 (with all three of the Playlists header's actions
|
||||
clipped), 69px at 390, and **0px** at 320 — the content was not
|
||||
degraded but gone. It goes to an overlay with a scrim when
|
||||
`available - panelWidth < 480`, where `available` is
|
||||
`.content-area`'s width and therefore already accounts for the
|
||||
sidebar's collapse.
|
||||
|
||||
Four things about it are load-bearing. **The mode is computed, not
|
||||
breakpointed**, because the panel's width is user state — drag-resizable
|
||||
200–500px and persisted — so a viewport breakpoint silently assumes the
|
||||
default 320 and is wrong by up to 180px in the direction that hurts;
|
||||
widening the panel at a fixed window size must flip it, and
|
||||
`queue-overlay-mode.test.ts` is written around exactly that. **480 is a
|
||||
judgement and says so**: there is no cliff to derive it from (the track
|
||||
list rescales continuously, 213px to 124px columns with no row
|
||||
overflow), so it is anchored to keep the default 1100px window inline
|
||||
and put every measured-broken case on the overlay side. **The scrim
|
||||
covers the content area only** — not the sidebar or the transport —
|
||||
because the queue is not modal, and it is subtle on a dark ramp by
|
||||
arithmetic rather than by accident (33,37,41 → 18,20,23). And **the
|
||||
overlay is a presentation, not a fork**: #55 asks for one component
|
||||
with two mount points, so the roving tab stop, Alt+Arrow reorder, drag
|
||||
reorder, selection semantics and `virtualizer.requestUpdate()` all come
|
||||
along untouched. Escape closes it and returns focus, and is attached
|
||||
only while the overlay is up — it is a dismissal, not a shortcut, which
|
||||
is why it is not a panel-scoped binding.
|
||||
|
||||
What this does **not** fix is `page-header` overflowing on its own:
|
||||
at 900×600 "New Smart Playlist" is still clipped to 114 of 162px with
|
||||
the queue *closed*. That is #69, and it cannot be fixed in
|
||||
`page-header` alone — actions arrive through `<slot name="actions">` as
|
||||
arbitrary light-DOM markup with their own handlers, so collapsing them
|
||||
into a "More actions" menu needs an actions *API* (data, not markup)
|
||||
across all three hosts that slot them.
|
||||
|
||||
**The phone section of `index.css` is last on purpose.** A media query
|
||||
adds no specificity, so a `@media (max-width: 599px)` block placed
|
||||
above the plain rules it overrides loses to them — which is how phase 1
|
||||
@@ -1918,6 +1980,79 @@ that corrects itself a moment later is worse than saying nothing. And
|
||||
the field, the direction and their persistence, so the control cannot
|
||||
disagree with the list.
|
||||
|
||||
**And an action is data, on that same rule: the header decides what
|
||||
fits, the host decides what happens.** Playlists slotted three buttons
|
||||
totalling 390px into a header that gets 700px at 900×600, so "New Smart
|
||||
Playlist" rendered **114 of its 162px** with the queue closed — and on a
|
||||
phone none of them could be reached at all, which is what #69 reported.
|
||||
A host passes `PageAction[]` (`{id, label, icon, onSelect, priority,
|
||||
drop?}`) and `page-header` renders each one as a button or as an item in
|
||||
one "More actions" menu.
|
||||
|
||||
**It could not have been a rule added in one place**, and that is a fact
|
||||
about the API rather than an effort estimate: actions used to arrive
|
||||
through `<slot name="actions">` as arbitrary light-DOM markup, and a
|
||||
component cannot move another component's light-DOM children into a
|
||||
dropdown and keep their behaviour — there is nothing generic in markup
|
||||
to render as a menu item. The slot survives for markup a data list
|
||||
cannot express, at the stated cost that **a slotted action does not
|
||||
collapse** and must therefore fit at 800×600.
|
||||
|
||||
Six things about it are load-bearing:
|
||||
|
||||
- **The fit is measured, never breakpointed.** A ResizeObserver drives
|
||||
it, and each pass starts from *all visible* and hides the
|
||||
lowest-priority action until it fits — so the collapsed set is a pure
|
||||
function of the current width rather than of how the window got
|
||||
there. A rule that only ever added to the set would never give a
|
||||
button back, and one that adjusted by a step would need a hysteresis
|
||||
band to stop it oscillating on the pixel where a button exactly fits.
|
||||
- **"Fits" means nothing is clipped, which is not the same as the
|
||||
header not overflowing.** The title can ellipsis, and the moment it
|
||||
can it absorbs the pressure: `scrollWidth` reports a header that fits
|
||||
perfectly while the heading reads "Playlis…". That is this bug moved
|
||||
from the button to the title, invisible to the same measurement that
|
||||
missed it the first time — so the heading's own truncation counts as
|
||||
not fitting, and an action is collapsed before the title gives way.
|
||||
Below that, at 320px, the title *is* what yields: the navigation also
|
||||
says which page you are on, and an action has nowhere else to be said.
|
||||
- **The measurement flips `hidden` on the rendered nodes rather than
|
||||
re-rendering between steps.** Reading `scrollWidth` forces layout,
|
||||
which is the point; awaiting a Lit update between steps instead lets
|
||||
the intermediate all-visible state paint, so the fix would flash the
|
||||
overflow it exists to prevent.
|
||||
- **Priority is what a *capability* costs, not what a button is worth.**
|
||||
New Playlist is highest because it is the **drop target** and a closed
|
||||
menu cannot be one; that is also why `PageAction.drop` carries the
|
||||
host's own `dragover`/`dragleave`/`drop` handlers rather than the
|
||||
header owning a notion of dropping, and why the affordance is simply
|
||||
absent from the overflow rather than approximated there.
|
||||
- **`aria-controls` names a panel that is always in the DOM** —
|
||||
`config-section`'s rule, and `wa-popup` hides it when inactive — and
|
||||
the keyboard model is `MenuKeyboard`, shared with every other menu in
|
||||
the app so this is not a second one.
|
||||
- **It is checked per button, because `layout-overflow.spec.ts` cannot
|
||||
see this.** That spec asserts the *shell* needs no sideways
|
||||
scrolling and passed on the broken build; clipping *inside* a
|
||||
component is invisible to it, which is exactly why the defect
|
||||
survived a spec named for it.
|
||||
`e2e/specs/header-action-overflow.spec.ts` measures each button
|
||||
against its header at 900×600, 800×600, 390×780 and 320×600, and
|
||||
asserts buttons **plus** menu account for every declared action —
|
||||
without that half it would pass vacuously on a build that renders no
|
||||
actions at all.
|
||||
|
||||
One thing it deliberately does **not** grow is a phone mode for the
|
||||
actions. `PHONE_COLUMN_IDS` is the precedent for "what is drawn and
|
||||
what can be sorted are different questions", but it exists because the
|
||||
track list's columns cannot be derived from a width; these can, and a
|
||||
second declaration of what a phone shows is a second thing to keep in
|
||||
step. What the header *does* state at phone width is one word: below
|
||||
600px the sort control's "Sort:" label is visually hidden — 172px of a
|
||||
320px header for a label the adjacent direction arrow implies — and it
|
||||
stays in the accessibility tree, because it is the select's accessible
|
||||
name and hiding it outright is `config-field`'s bug one component over.
|
||||
|
||||
**The header search box is view-scoped, and now says so.** It sits in
|
||||
the app header and reads as global; typing `tide` on Playlists answered
|
||||
"No playlists match your search" with three *Tideline* tracks in the
|
||||
@@ -2307,6 +2442,23 @@ Pre-commit hooks verify generated code is fresh — always run `make generate` a
|
||||
mistyped `feat` ships a minor version. `make release-dry` answers "what
|
||||
would this merge release" without pushing.
|
||||
|
||||
**The analyzer reads the type and ignores the scope, so a CI-only change
|
||||
is `ci:` and never `fix(ci):`.** The scope is decoration; `fix` is a
|
||||
patch whatever is in the brackets. Two commits touching nothing but
|
||||
`.gitea/workflows/unclaim.yml` were written `fix(ci):` and cut `v0.2.1`
|
||||
and `v0.2.2` — real releases, published to Arch, Homebrew and the APK
|
||||
registry, containing no user-facing change. They were left in place
|
||||
rather than deleted, because a version that vanishes is worse for
|
||||
whoever pulled it than one that turns out to be empty.
|
||||
|
||||
**The blast radius is bigger than the version number**, which is what
|
||||
makes this worth a paragraph. A merge to `main` starts two workflows;
|
||||
if `release.yml` then pushes a tag, that tag push starts **four more**
|
||||
(`arch-package`, `homebrew-formula`, `android-apk`, `desktop-assets`) —
|
||||
on a runner with capacity 1, where the APK build alone is tens of
|
||||
minutes. `make release-dry` before merging is how you find out, and it
|
||||
is cheaper than every one of those.
|
||||
|
||||
**`@semantic-release/github` is not in that config and must not be.**
|
||||
Gitea's API is `/api/v1` and is not GitHub's surface, so
|
||||
`@semantic-release/exec` calls `scripts/gitea-release.sh` instead — one
|
||||
|
||||
@@ -13,13 +13,31 @@ const (
|
||||
// enforces this at runtime; it is also the floor below which a
|
||||
// reported size is treated as bogus and not persisted.
|
||||
//
|
||||
// 800x600 is where the shell was measured to still work, rather
|
||||
// than a round number: below ~780 the header's subtitle wraps and
|
||||
// pushes the title out of the 4em top bar, and below ~600 tall the
|
||||
// eleven sidebar items no longer fit at once. The previous
|
||||
// 512x384 was aspirational — at 700x480 the sidebar overflowed
|
||||
// behind the player bar with no scroll and Settings and Jobs could
|
||||
// not be reached at all.
|
||||
// **Both reasons this comment used to give have expired**, and the
|
||||
// value is right for a third one. It said the floor was 800x600
|
||||
// because "below ~780 the header's subtitle wraps and pushes the
|
||||
// title out of the 4em top bar" and "below ~600 tall the eleven
|
||||
// sidebar items no longer fit at once". Neither mechanism can
|
||||
// happen now: the subtitle is display:none from 899px down
|
||||
// (index.css), and the sidebar host is overflow-y:auto — measured
|
||||
// at 600x460, its scrollHeight is 434 against a 332px client and
|
||||
// Settings is reachable after scrolling. A floor defended by two
|
||||
// mechanisms that no longer exist is a number nobody can argue
|
||||
// with, which is worse than either answer.
|
||||
//
|
||||
// It stays 800x600 because that is where the *desktop* chrome
|
||||
// stops being comfortable — the Compact band of plan 018's size
|
||||
// matrix (#24) — and not because the app breaks below it. It does
|
||||
// not: under 600px wide the phone layout takes over (bottom-nav,
|
||||
// no sidebar) and the shell fits 320px exactly, which is what
|
||||
// makes this a comfort floor rather than a correctness one, and
|
||||
// why a very small window reflows instead of becoming a
|
||||
// mini-player (#12 is a second always-on-top window, not a mode of
|
||||
// this one).
|
||||
//
|
||||
// The previous 512x384 was aspirational — at 700x480 the sidebar
|
||||
// overflowed behind the player bar with no scroll and Settings and
|
||||
// Jobs could not be reached at all.
|
||||
MinWidth = 800
|
||||
// MinHeight is the smallest allowed window height in pixels.
|
||||
MinHeight = 600
|
||||
|
||||
@@ -82,6 +82,100 @@ func TestPruneStaleLocalCrossReferences(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestPruneClearsInLibraryWithNoLocalID covers the fixed point: a row
|
||||
// carrying in_library with a NULL local_*_id. The upsert's conflict
|
||||
// clause is `in_library = MAX(in_library, excluded.in_library)`, so it
|
||||
// can only ever raise the flag, and this pass used to be gated on the id
|
||||
// being present — which meant nothing in the app could clear such a row,
|
||||
// ever. It is asserted for all three entity types because the gate was
|
||||
// written once and used three times, so a fix applied to one is a fix
|
||||
// that looks complete.
|
||||
//
|
||||
// The rows are seeded with raw SQL rather than through seedIndexResult
|
||||
// deliberately: upsertBatch writes a zero LocalArtistID as literal 0,
|
||||
// not NULL, and 0 satisfies `IS NOT NULL` — so the old gate already
|
||||
// caught that shape and a fixture built through the upsert cannot
|
||||
// reproduce this at all. NULL is what the artifact importer and any
|
||||
// older writer leave behind, the column being nullable with no default.
|
||||
func TestPruneClearsInLibraryWithNoLocalID(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
db := database.NewTestDB(t)
|
||||
si := NewSearchIndex(db, nil, nil, slog.Default())
|
||||
|
||||
// A genuinely owned artist, to prove the wider gate does not simply
|
||||
// clear everything it now looks at.
|
||||
database.InsertTestTrack(t, db, database.TestTrack{
|
||||
FilePath: "/music/owned.mp3",
|
||||
Artist: "Owned",
|
||||
})
|
||||
|
||||
artist, err := db.Queries.GetArtistByName(t.Context(), "Owned")
|
||||
if err != nil {
|
||||
t.Fatalf("read seeded artist: %v", err)
|
||||
}
|
||||
|
||||
seedIndexResult(t, db, SearchIndexResult{
|
||||
EntityType: EntityArtist,
|
||||
MBID: testMBID("owned"),
|
||||
Title: "Owned",
|
||||
ArtistName: "Owned",
|
||||
ArtistMBID: testMBID("owned"),
|
||||
InLibrary: true,
|
||||
LocalArtistID: artist.ID,
|
||||
})
|
||||
|
||||
orphans := []struct {
|
||||
name string
|
||||
entityType string
|
||||
mbid string
|
||||
}{
|
||||
{"artist", EntityArtist, "orphan-artist"},
|
||||
{"release group", EntityReleaseGroup, "orphan-release-group"},
|
||||
{"recording", EntityRecording, "orphan-recording"},
|
||||
}
|
||||
|
||||
for _, o := range orphans {
|
||||
if _, err := db.ExecContext(
|
||||
`INSERT INTO explore_index
|
||||
(entity_type, mbid, title, artist_name, artist_mbid,
|
||||
in_library,
|
||||
local_artist_id, local_release_group_id, local_recording_id)
|
||||
VALUES (?, ?, ?, ?, ?, 1, ?, ?, ?)`,
|
||||
dbEntityType(o.entityType), dbMBID(testMBID(o.mbid)), o.name, o.name,
|
||||
dbMBID(testMBID(o.mbid)),
|
||||
nil, nil, nil,
|
||||
); err != nil {
|
||||
t.Fatalf("seed %s orphan: %v", o.name, err)
|
||||
}
|
||||
}
|
||||
|
||||
si.pruneStaleLocalCrossReferences()
|
||||
|
||||
inLibrary := func(t *testing.T, mbid string) int {
|
||||
t.Helper()
|
||||
|
||||
var flag int
|
||||
if err := db.QueryRowWriter(
|
||||
"SELECT in_library FROM explore_index WHERE mbid = ?", dbMBID(mbid),
|
||||
).Scan(&flag); err != nil {
|
||||
t.Fatalf("read in_library for %q: %v", mbid, err)
|
||||
}
|
||||
|
||||
return flag
|
||||
}
|
||||
|
||||
for _, o := range orphans {
|
||||
if got := inLibrary(t, testMBID(o.mbid)); got != 0 {
|
||||
t.Errorf("%s with a NULL local id: in_library = %d, want 0", o.name, got)
|
||||
}
|
||||
}
|
||||
|
||||
if got := inLibrary(t, testMBID("owned")); got != 1 {
|
||||
t.Errorf("owned artist: in_library = %d, want 1 (it still has a file)", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestUnenrichedLibraryArtistMBIDs_OrdersByOwnedTrackCount verifies the
|
||||
// backfill queue prioritizes artists by how many tracks the user actually
|
||||
// owns, not by how many duplicate-mbid artist rows happen to exist (the
|
||||
|
||||
@@ -2562,6 +2562,19 @@ func (si *SearchIndex) PopulateLocalCrossReferences() {
|
||||
// The row itself is left in place (it may still be part of the shipped
|
||||
// catalog, just no longer owned) — only the "this is mine" bookkeeping
|
||||
// is cleared.
|
||||
//
|
||||
// It is gated on the flag *or* the id, not on the id alone. Gated on
|
||||
// the id, `in_library = 1 AND local_*_id IS NULL` is a fixed point: the
|
||||
// upsert can only ever raise the flag and this pass skipped such a row
|
||||
// by construction, so nothing in the app could clear it — a row claiming
|
||||
// to be owned, permanently, with no local row to check the claim
|
||||
// against. Nothing in the tree writes that shape today
|
||||
// (collectLibraryEntities sets both together), which is exactly why it
|
||||
// is worth closing now: the exposure is a database written by an older
|
||||
// version, and the next writer that sets the flag without an id, which
|
||||
// nothing structurally prevents. A NULL id fails the existence test on
|
||||
// its own, so the wider gate needs no second clause to say what "not
|
||||
// owned" means.
|
||||
func (si *SearchIndex) pruneStaleLocalCrossReferences() {
|
||||
type prune struct {
|
||||
entityType string
|
||||
@@ -2594,7 +2607,8 @@ func (si *SearchIndex) pruneStaleLocalCrossReferences() {
|
||||
result, err := si.db.ExecContext(
|
||||
`UPDATE explore_index
|
||||
SET in_library = 0, `+p.column+` = NULL
|
||||
WHERE entity_type = ? AND `+p.column+` IS NOT NULL
|
||||
WHERE entity_type = ?
|
||||
AND (`+p.column+` IS NOT NULL OR in_library = 1)
|
||||
AND NOT EXISTS (`+p.exists+`)`,
|
||||
dbEntityType(p.entityType),
|
||||
)
|
||||
|
||||
@@ -1,6 +1,12 @@
|
||||
import { test, expect } from '../support/fixtures.js';
|
||||
import type { Page } from '@playwright/test';
|
||||
|
||||
/**
|
||||
* How far the scroll test scrolls. One constant, because the guard and
|
||||
* the assertion have to agree about it — they did not, which is #133.
|
||||
*/
|
||||
const SCROLL_TARGET = 80;
|
||||
|
||||
/**
|
||||
* Plan 007 phase 5: expanding an album shows its tracks.
|
||||
*
|
||||
@@ -104,20 +110,27 @@ test.describe('the album dropdown', () => {
|
||||
await app.setViewportSize({ width: 900, height: 600 });
|
||||
|
||||
try {
|
||||
await expect.poll(() => scrollRange(app)).toMatchObject({
|
||||
scrollable: true,
|
||||
overflowY: 'auto',
|
||||
});
|
||||
// Wait for the range the assertion below actually needs, not for
|
||||
// "scrollable at all" (#133). The guard used to be
|
||||
// `scrollHeight > clientHeight + 40` while the next line asks to
|
||||
// reach 80, so any range in 41-79 satisfied it and could not
|
||||
// satisfy the assertion — and the grid passes through exactly
|
||||
// that while it settles, because it recomputes its columns after
|
||||
// the resize rather than during it. The settled range here is
|
||||
// 330, so this waits rather than weakening anything.
|
||||
await expect
|
||||
.poll(() => scrollRange(app))
|
||||
.toMatchObject({ room: true, overflowY: 'auto' });
|
||||
|
||||
await app.evaluate(() => {
|
||||
await app.evaluate((target) => {
|
||||
const sc = document
|
||||
.querySelector('cover-grid')
|
||||
?.shadowRoot?.querySelector('.grid-scroll-container');
|
||||
|
||||
if (sc) sc.scrollTop = 80;
|
||||
});
|
||||
if (sc) sc.scrollTop = target;
|
||||
}, SCROLL_TARGET);
|
||||
|
||||
expect(await scrollTop(app)).toBe(80);
|
||||
expect(await scrollTop(app)).toBe(SCROLL_TARGET);
|
||||
|
||||
// And the dropdown it opens is on screen, wherever the manager
|
||||
// decides that leaves the scroll. It is *not* "the position is
|
||||
@@ -250,16 +263,19 @@ async function closeDropdown(app: Page): Promise<void> {
|
||||
|
||||
/** Whether the grid can scroll at all, which decides if a probe can move. */
|
||||
async function scrollRange(app: Page) {
|
||||
return app.evaluate(() => {
|
||||
return app.evaluate((target) => {
|
||||
const sc = document
|
||||
.querySelector('cover-grid')
|
||||
?.shadowRoot?.querySelector('.grid-scroll-container');
|
||||
|
||||
return {
|
||||
scrollable: !!sc && sc.scrollHeight > sc.clientHeight + 40,
|
||||
// `room` is the precondition of the assertion that follows it:
|
||||
// enough range to actually reach the target. A threshold below
|
||||
// what the caller depends on is not a guard.
|
||||
room: !!sc && sc.scrollHeight - sc.clientHeight >= target,
|
||||
overflowY: sc ? getComputedStyle(sc).overflowY : '',
|
||||
};
|
||||
});
|
||||
}, SCROLL_TARGET);
|
||||
}
|
||||
|
||||
async function scrollTop(app: Page): Promise<number> {
|
||||
|
||||
@@ -0,0 +1,318 @@
|
||||
import { test, expect } from '../support/fixtures.js';
|
||||
|
||||
/**
|
||||
* #69: the Playlists header's buttons could not be reached.
|
||||
*
|
||||
* Three text buttons — Import (91px), New Playlist (122px), New Smart
|
||||
* Playlist (162px), 390px in total — inside a header that gets 700px at
|
||||
* 900×600. "New Smart Playlist" rendered **114 of its 162px**, and at
|
||||
* phone width the Android report was the plain version of it: you
|
||||
* cannot scroll to reach them, and scrolling is not how page controls
|
||||
* should be exposed anyway.
|
||||
*
|
||||
* **`layout-overflow.spec.ts` passes on the broken build**, which is why
|
||||
* this file exists rather than a case being added there. That spec
|
||||
* asserts the *shell* needs no sideways scrolling; clipping *inside* a
|
||||
* component is invisible to it. So the measurement here is per-button
|
||||
* and per-header, against the widths the app promises.
|
||||
*
|
||||
* Plan 018's size matrix is the promise being kept: **no action is ever
|
||||
* unreachable at any supported size.** These are its three bands.
|
||||
*/
|
||||
|
||||
const VIEWPORTS = [
|
||||
// Desktop's worst case, and not the enforced minimum: the sidebar
|
||||
// collapses to icons *below* 900, so the content area is 843px at 899
|
||||
// and 700px at 900. Testing "the minimum" and stopping misses it.
|
||||
{ name: '900×600 (widest sidebar, narrowest content)', width: 900, height: 600 },
|
||||
{ name: '800×600 (the enforced minimum)', width: 800, height: 600 },
|
||||
{ name: '390×780 (phone)', width: 390, height: 780 },
|
||||
// WCAG 1.4.10's reflow target, which plan 018 promises the app fits.
|
||||
{ name: '320×600 (400% zoom)', width: 320, height: 600 },
|
||||
];
|
||||
|
||||
/** Every action the Playlists header can offer, in declared order. */
|
||||
const ACTIONS = ['Import', 'New Playlist', 'New Smart Playlist'];
|
||||
|
||||
/**
|
||||
* What the header is actually rendering, measured rather than inferred.
|
||||
*
|
||||
* A shadow query is the wrong tool for *asserting* — that is what
|
||||
* `getByRole` below is for — but it is the right one for a measurement,
|
||||
* because the number this issue is about (a button 48px wider than the
|
||||
* box holding it) is not in the accessibility tree at all.
|
||||
*/
|
||||
const headerFit = (page: import('@playwright/test').Page) =>
|
||||
page.evaluate(() => {
|
||||
const root = document
|
||||
.querySelector('[data-testid="main-content"] playlist-view')
|
||||
?.shadowRoot?.querySelector('page-header')?.shadowRoot;
|
||||
|
||||
if (!root) return null;
|
||||
|
||||
const header = root.querySelector<HTMLElement>('.page-header')!;
|
||||
const box = header.getBoundingClientRect();
|
||||
const title = root.querySelector<HTMLElement>('h1')!;
|
||||
|
||||
const clipped = [
|
||||
...root.querySelectorAll<HTMLElement>('.action, .more-button'),
|
||||
]
|
||||
.filter((b) => !b.hidden)
|
||||
.filter((b) => {
|
||||
const r = b.getBoundingClientRect();
|
||||
|
||||
return r.right > box.right + 1 || r.left < box.left - 1;
|
||||
})
|
||||
.map((b) => b.dataset['actionId'] ?? 'more');
|
||||
|
||||
return {
|
||||
overflow: header.scrollWidth - header.clientWidth,
|
||||
clipped,
|
||||
titleTruncated: title.scrollWidth > title.clientWidth + 1,
|
||||
buttons: [...root.querySelectorAll<HTMLElement>('.action')]
|
||||
.filter((b) => !b.hidden)
|
||||
.map((b) => b.textContent?.trim() ?? ''),
|
||||
menu: [
|
||||
...root.querySelectorAll('#page-header-overflow wa-dropdown-item'),
|
||||
].map((i) => i.textContent?.trim() ?? ''),
|
||||
};
|
||||
});
|
||||
|
||||
test.describe('the page header never clips an action', () => {
|
||||
test.beforeEach(async ({ app }) => {
|
||||
await app.getByTestId('nav-playlists').click();
|
||||
await expect(app.getByTestId('main-content')).toHaveAttribute(
|
||||
'data-active-view',
|
||||
'playlists',
|
||||
);
|
||||
});
|
||||
|
||||
test.afterEach(async ({ app }) => {
|
||||
await app.setViewportSize({ width: 1280, height: 800 });
|
||||
});
|
||||
|
||||
for (const vp of VIEWPORTS) {
|
||||
test(`every action is reachable at ${vp.name}`, async ({ app }) => {
|
||||
await app.setViewportSize({ width: vp.width, height: vp.height });
|
||||
|
||||
// Polled: the fit is decided by a ResizeObserver, so it settles a
|
||||
// frame after the resize rather than with it.
|
||||
await expect
|
||||
.poll(async () => (await headerFit(app))?.clipped)
|
||||
.toEqual([]);
|
||||
|
||||
const fit = (await headerFit(app))!;
|
||||
|
||||
expect(fit.overflow).toBeLessThanOrEqual(0);
|
||||
|
||||
// Between them, buttons and menu account for all three. This is
|
||||
// the assertion the issue asks for: not "it fits" but "nothing
|
||||
// was dropped to make it fit".
|
||||
expect([...fit.buttons, ...fit.menu].sort()).toEqual([...ACTIONS].sort());
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* The title gives way before an action does.
|
||||
*
|
||||
* Once the heading can ellipsis it absorbs the pressure, and
|
||||
* `scrollWidth` then reports a header that fits perfectly while the
|
||||
* heading reads "Playlis…" — this issue's own failure mode moved from
|
||||
* the button to the title, and invisible to exactly the measurement
|
||||
* that missed it the first time. At the desktop sizes there is always
|
||||
* an action to collapse instead.
|
||||
*/
|
||||
test('does not truncate the heading to keep a button', async ({ app }) => {
|
||||
for (const vp of VIEWPORTS.slice(0, 2)) {
|
||||
await app.setViewportSize({ width: vp.width, height: vp.height });
|
||||
|
||||
await expect
|
||||
.poll(async () => (await headerFit(app))?.titleTruncated)
|
||||
.toBe(false);
|
||||
}
|
||||
});
|
||||
|
||||
/**
|
||||
* Asserted through the accessibility tree, never a shadow query. An
|
||||
* overflow menu is exactly the shape that grows a nameless control,
|
||||
* and this repo has shipped one four times — most recently the
|
||||
* queue's own close button.
|
||||
*/
|
||||
test('the overflow is a named control that opens a named menu', async ({
|
||||
app,
|
||||
}) => {
|
||||
await app.setViewportSize({ width: 900, height: 600 });
|
||||
|
||||
const more = app.getByRole('button', { name: 'More actions' });
|
||||
|
||||
await expect(more).toBeVisible();
|
||||
await expect(more).toHaveAttribute('aria-expanded', 'false');
|
||||
|
||||
await more.click();
|
||||
|
||||
await expect(more).toHaveAttribute('aria-expanded', 'true');
|
||||
|
||||
const menu = app.getByRole('menu', { name: 'More actions' });
|
||||
|
||||
await expect(menu).toBeVisible();
|
||||
|
||||
// Collapsed at 900×600: Import (lowest priority) and New Smart
|
||||
// Playlist. New Playlist stays a button because it is the drop
|
||||
// target, and a closed menu cannot be one.
|
||||
await expect(
|
||||
menu.getByRole('menuitem', { name: 'Import' }),
|
||||
).toBeVisible();
|
||||
await expect(
|
||||
app.getByRole('button', { name: 'New Playlist', exact: true }),
|
||||
).toBeVisible();
|
||||
});
|
||||
|
||||
/**
|
||||
* The phone case is the original report. Every action is in the menu
|
||||
* at 390px, and the menu is reachable by name — which is the whole of
|
||||
* "these need to be reachable in a sensible way".
|
||||
*/
|
||||
test('offers every action from the menu on a phone', async ({ app }) => {
|
||||
await app.setViewportSize({ width: 390, height: 780 });
|
||||
|
||||
const more = app.getByRole('button', { name: 'More actions' });
|
||||
|
||||
await expect(more).toBeVisible();
|
||||
await more.click();
|
||||
|
||||
const menu = app.getByRole('menu', { name: 'More actions' });
|
||||
|
||||
for (const label of ACTIONS) {
|
||||
await expect(menu.getByRole('menuitem', { name: label })).toBeVisible();
|
||||
}
|
||||
});
|
||||
|
||||
/**
|
||||
* Escape closes it and focus goes back to the trigger — `MenuKeyboard`
|
||||
* is shared with every other menu in the app precisely so this is not
|
||||
* a second keyboard model, and this is what proves it was wired up
|
||||
* rather than merely imported.
|
||||
*/
|
||||
test('takes the keyboard, and gives it back', async ({ app }) => {
|
||||
await app.setViewportSize({ width: 900, height: 600 });
|
||||
|
||||
const more = app.getByRole('button', { name: 'More actions' });
|
||||
|
||||
await more.click();
|
||||
|
||||
const menu = app.getByRole('menu', { name: 'More actions' });
|
||||
|
||||
await expect(menu).toBeVisible();
|
||||
|
||||
// The first item takes focus on open. `wa-dropdown-item` sets its
|
||||
// own role in its own first update, so this is polled rather than
|
||||
// read: a query at the host's updateComplete finds nothing, which
|
||||
// reads exactly like a menu that refused to take focus.
|
||||
await expect
|
||||
.poll(async () =>
|
||||
app.evaluate(() => {
|
||||
// Stops where `MenuKeyboard`'s own `deepActiveElement` stops:
|
||||
// on the *host* whose shadow root has no active element.
|
||||
// Descending unconditionally lands inside the focused
|
||||
// `wa-dropdown-item`'s own shadow root, where nothing is
|
||||
// focused — which reads exactly like a menu that refused the
|
||||
// keyboard, on a build where it did not.
|
||||
let el = document.activeElement;
|
||||
|
||||
while (el?.shadowRoot?.activeElement) el = el.shadowRoot.activeElement;
|
||||
|
||||
return el?.textContent?.trim() ?? null;
|
||||
}),
|
||||
)
|
||||
.toBe('Import');
|
||||
|
||||
await app.keyboard.press('Escape');
|
||||
|
||||
await expect(more).toHaveAttribute('aria-expanded', 'false');
|
||||
await expect(more).toBeFocused();
|
||||
});
|
||||
|
||||
/**
|
||||
* New Playlist is a drop target, and declaring it as data must not
|
||||
* take that away — which is why a `PageAction` carries the drop
|
||||
* handlers rather than the header owning a notion of dropping.
|
||||
*
|
||||
* Nothing covered this before, in either tier, and it is the one
|
||||
* behaviour the migration could plausibly have destroyed silently:
|
||||
* dragging still *looks* fine against a button that no longer
|
||||
* accepts anything.
|
||||
*/
|
||||
test('New Playlist still accepts a dropped track', async ({ app }) => {
|
||||
await app.setViewportSize({ width: 1280, height: 800 });
|
||||
|
||||
const button = app.getByRole('button', {
|
||||
name: 'New Playlist',
|
||||
exact: true,
|
||||
});
|
||||
|
||||
await expect(button).toBeVisible();
|
||||
|
||||
const result = await app.evaluate(async () => {
|
||||
const view = document.querySelector(
|
||||
'[data-testid="main-content"] playlist-view',
|
||||
)!;
|
||||
const target = view.shadowRoot!
|
||||
.querySelector('page-header')!
|
||||
.shadowRoot!.querySelector('[data-testid="page-action-new-playlist"]')!;
|
||||
|
||||
const data = new DataTransfer();
|
||||
|
||||
data.setData(
|
||||
'application/x-yj-tracks',
|
||||
JSON.stringify({ filePaths: ['/tmp/dropped.mp3'] }),
|
||||
);
|
||||
|
||||
const fire = (type: string) =>
|
||||
target.dispatchEvent(
|
||||
new DragEvent(type, {
|
||||
bubbles: true,
|
||||
cancelable: true,
|
||||
dataTransfer: data,
|
||||
}),
|
||||
);
|
||||
|
||||
fire('dragover');
|
||||
await new Promise((r) => setTimeout(r, 50));
|
||||
|
||||
// The affordance is the host's state reaching the header's
|
||||
// button, which is the half a plain handler call would not prove.
|
||||
const highlighted = target.classList.contains('drag-over');
|
||||
|
||||
fire('drop');
|
||||
await new Promise((r) => setTimeout(r, 200));
|
||||
|
||||
return {
|
||||
highlighted,
|
||||
opened: view.shadowRoot!.querySelector('.create-form') !== null,
|
||||
};
|
||||
});
|
||||
|
||||
expect(result).toEqual({ highlighted: true, opened: true });
|
||||
|
||||
// Leave the view as it was found.
|
||||
await app.keyboard.press('Escape');
|
||||
});
|
||||
|
||||
/**
|
||||
* An action given back when the window widens again. The collapsed
|
||||
* set is a function of the current width and not of how it got there
|
||||
* — a rule that only ever *added* to it would never widen.
|
||||
*/
|
||||
test('gives the buttons back when the window grows', async ({ app }) => {
|
||||
await app.setViewportSize({ width: 390, height: 780 });
|
||||
|
||||
await expect.poll(async () => (await headerFit(app))?.buttons).toEqual([]);
|
||||
|
||||
await app.setViewportSize({ width: 1440, height: 900 });
|
||||
|
||||
await expect
|
||||
.poll(async () => (await headerFit(app))?.buttons)
|
||||
.toEqual(ACTIONS);
|
||||
await expect.poll(async () => (await headerFit(app))?.menu).toEqual([]);
|
||||
});
|
||||
});
|
||||
@@ -26,6 +26,12 @@ const MIN_VIEWPORT = { width: 800, height: 600 };
|
||||
const VIEWPORTS = [
|
||||
{ name: '1440×900', width: 1440, height: 900 },
|
||||
{ name: '1024×768', width: 1024, height: 768 },
|
||||
// Not the minimum, and that is the point (#24). The sidebar collapses
|
||||
// to icons *below* 900, so the main panel is 843px at 899 and 700px
|
||||
// at 900 — the narrowest content area any desktop width produces is
|
||||
// here, not at the enforced floor. A list that stopped at the minimum
|
||||
// was missing its own worst case.
|
||||
{ name: '900×600 (the widest sidebar, so the narrowest content)', width: 900, height: 600 },
|
||||
{ name: `the minimum (${MIN_VIEWPORT.width}×${MIN_VIEWPORT.height})`, ...MIN_VIEWPORT },
|
||||
];
|
||||
|
||||
|
||||
@@ -0,0 +1,187 @@
|
||||
import { test, expect } from '../support/fixtures.js';
|
||||
|
||||
/**
|
||||
* #24 — the queue panel does not take the page's width away from it.
|
||||
*
|
||||
* The panel is `flex-shrink: 0` in the flow of `.content-area`, so an
|
||||
* open queue used to be paid for by the main panel. Measured on
|
||||
* Playlists before the fix:
|
||||
*
|
||||
* | viewport | main panel |
|
||||
* |---|---|
|
||||
* | 900×600 | 379px — all three header actions clipped |
|
||||
* | 390×780 | 69px |
|
||||
* | 320×600 | **0px** |
|
||||
*
|
||||
* **900×600 is the worst desktop case, not the 800×600 minimum**, and
|
||||
* that is the trap this file exists to keep closed: the sidebar
|
||||
* collapses to icons *below* 900, so the main panel is 843px at 899 and
|
||||
* 700px at 900. A spec that checks "the minimum" and stops has not
|
||||
* checked the worst case — which is what every viewport list in this
|
||||
* suite did before this.
|
||||
*
|
||||
* These assert the *content's* width rather than the panel's mode
|
||||
* wherever they can, because the mode is the mechanism and the width is
|
||||
* the complaint.
|
||||
*/
|
||||
|
||||
/** The bands from plan 018's size matrix, plus the pixel above the collapse. */
|
||||
const BANDS = [
|
||||
{ name: 'a wide desktop (1280×800)', width: 1280, height: 800, inline: true },
|
||||
{ name: 'the default window (1100×720)', width: 1100, height: 720, inline: true },
|
||||
{ name: 'a laptop (1024×768)', width: 1024, height: 768, inline: true },
|
||||
{ name: 'the worst desktop width (900×600)', width: 900, height: 600, inline: false },
|
||||
{ name: 'the enforced minimum (800×600)', width: 800, height: 600, inline: false },
|
||||
{ name: 'a phone (390×780)', width: 390, height: 780, inline: false },
|
||||
{ name: '400% zoom (320×600)', width: 320, height: 600, inline: false },
|
||||
];
|
||||
|
||||
/**
|
||||
* How much room the content has, and whether the shell needs scrolling
|
||||
* to reach any of itself.
|
||||
*/
|
||||
const shellGeometry = (page: import('@playwright/test').Page) =>
|
||||
page.evaluate(() => {
|
||||
const main = document.querySelector('#main-content')!.getBoundingClientRect();
|
||||
const panel = document.querySelector('#queue-panel')!;
|
||||
|
||||
return {
|
||||
mainWidth: Math.round(main.width),
|
||||
overlay: panel.hasAttribute('overlay'),
|
||||
open: panel.hasAttribute('open'),
|
||||
bodyScrollWidth: document.body.scrollWidth,
|
||||
bodyClientWidth: document.body.clientWidth,
|
||||
};
|
||||
});
|
||||
|
||||
async function openQueue(page: import('@playwright/test').Page) {
|
||||
const toggle = page.locator('#queue-button');
|
||||
|
||||
if ((await toggle.getAttribute('aria-expanded')) !== 'true') {
|
||||
await toggle.click();
|
||||
}
|
||||
|
||||
await expect(toggle).toHaveAttribute('aria-expanded', 'true');
|
||||
}
|
||||
|
||||
test.describe('an open queue leaves the content its width', () => {
|
||||
for (const band of BANDS) {
|
||||
test(`at ${band.name}`, async ({ app }) => {
|
||||
await app.setViewportSize({ width: band.width, height: band.height });
|
||||
await openQueue(app);
|
||||
|
||||
// The mode is settled by a ResizeObserver, so poll rather than
|
||||
// read once: a single read races the resize and reports the
|
||||
// previous viewport's answer.
|
||||
await expect
|
||||
.poll(async () => (await shellGeometry(app)).overlay)
|
||||
.toBe(!band.inline);
|
||||
|
||||
const geo = await shellGeometry(app);
|
||||
|
||||
// The floor is the point of the whole issue. Inline, the queue is
|
||||
// affordable and the content keeps the rest; as an overlay the
|
||||
// content keeps *everything*, which is what makes 0px at 320
|
||||
// impossible rather than merely unlikely.
|
||||
expect(geo.mainWidth).toBeGreaterThanOrEqual(320);
|
||||
|
||||
if (!band.inline) {
|
||||
expect(geo.mainWidth).toBeGreaterThanOrEqual(
|
||||
Math.min(band.width, 320),
|
||||
);
|
||||
}
|
||||
|
||||
// And opening the queue must not make the shell overflow.
|
||||
expect(geo.bodyScrollWidth).toBeLessThanOrEqual(geo.bodyClientWidth);
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
test.describe('an overlaid queue says it is over the content', () => {
|
||||
test.beforeEach(async ({ app }) => {
|
||||
await app.setViewportSize({ width: 900, height: 600 });
|
||||
});
|
||||
|
||||
test('draws a scrim and closes when it is clicked', async ({ app }) => {
|
||||
await openQueue(app);
|
||||
|
||||
const panel = app.locator('#queue-panel');
|
||||
|
||||
await expect(panel).toHaveAttribute('overlay', '');
|
||||
|
||||
// The scrim is `aria-hidden` on purpose — it is a dismissal target,
|
||||
// and the named routes out are the close button and Escape — so it
|
||||
// is located structurally rather than by role.
|
||||
await panel.evaluate((el) =>
|
||||
el.shadowRoot!.querySelector<HTMLElement>('.scrim')!.click(),
|
||||
);
|
||||
|
||||
await expect(app.locator('#queue-button')).toHaveAttribute(
|
||||
'aria-expanded',
|
||||
'false',
|
||||
);
|
||||
});
|
||||
|
||||
/**
|
||||
* `getByRole`, not a shadow-root query: this repo has shipped a
|
||||
* nameless control three times, and a drawer with a scrim is exactly
|
||||
* the shape that grows a fourth.
|
||||
*/
|
||||
test('offers a named close button', async ({ app }) => {
|
||||
await openQueue(app);
|
||||
|
||||
const close = app.getByRole('button', { name: 'Close queue' });
|
||||
|
||||
await expect(close).toBeVisible();
|
||||
await close.click();
|
||||
|
||||
await expect(app.locator('#queue-button')).toHaveAttribute(
|
||||
'aria-expanded',
|
||||
'false',
|
||||
);
|
||||
});
|
||||
|
||||
test('closes on Escape and gives focus back to the toggle', async ({
|
||||
app,
|
||||
}) => {
|
||||
const toggle = app.locator('#queue-button');
|
||||
|
||||
await toggle.focus();
|
||||
await toggle.click();
|
||||
await expect(toggle).toHaveAttribute('aria-expanded', 'true');
|
||||
|
||||
await app.keyboard.press('Escape');
|
||||
|
||||
await expect(toggle).toHaveAttribute('aria-expanded', 'false');
|
||||
await expect(toggle).toBeFocused();
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* The inline panel is the mode that already worked, and the one every
|
||||
* other queue spec is written against. It keeps its resize handle and
|
||||
* gains none of the overlay's chrome.
|
||||
*/
|
||||
test.describe('a wide window keeps the queue beside the content', () => {
|
||||
test('no scrim, no close button, and the content is narrower', async ({
|
||||
app,
|
||||
}) => {
|
||||
await app.setViewportSize({ width: 1280, height: 800 });
|
||||
|
||||
const widthWithoutQueue = (await shellGeometry(app)).mainWidth;
|
||||
|
||||
await openQueue(app);
|
||||
|
||||
await expect(app.locator('#queue-panel')).not.toHaveAttribute(
|
||||
'overlay',
|
||||
'',
|
||||
);
|
||||
|
||||
const geo = await shellGeometry(app);
|
||||
|
||||
expect(geo.mainWidth).toBeLessThan(widthWithoutQueue);
|
||||
await expect(
|
||||
app.getByRole('button', { name: 'Close queue' }),
|
||||
).toHaveCount(0);
|
||||
});
|
||||
});
|
||||
@@ -231,6 +231,14 @@ body div.sidebar {
|
||||
display: flex;
|
||||
overflow: hidden;
|
||||
contain: layout style;
|
||||
|
||||
/* The containing block for the queue panel's overlay mode (plan
|
||||
018, #24), which spans this box rather than taking width from
|
||||
the main panel beside it. `contain: layout` already establishes
|
||||
one; this says so on purpose, so that removing the containment
|
||||
for a paint reason does not silently reparent the overlay to the
|
||||
viewport. */
|
||||
position: relative;
|
||||
}
|
||||
|
||||
.main-panel {
|
||||
|
||||
@@ -0,0 +1 @@
|
||||
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 448 512"><!--! Font Awesome Free 7.3.1 by @fontawesome - https://fontawesome.com License - https://fontawesome.com/license/free (Icons: CC BY 4.0, Fonts: SIL OFL 1.1, Code: MIT License) Copyright 2026 Fonticons, Inc. --><path fill="currentColor" d="M0 256a56 56 0 1 1 112 0 56 56 0 1 1 -112 0zm168 0a56 56 0 1 1 112 0 56 56 0 1 1 -112 0zm224-56a56 56 0 1 1 0 112 56 56 0 1 1 0-112z"/></svg>
|
||||
|
After Width: | Height: | Size: 443 B |
@@ -3,6 +3,7 @@ import { customElement, state } from 'lit/decorators.js';
|
||||
import '@awesome.me/webawesome/dist/components/icon/icon.js';
|
||||
import '@awesome.me/webawesome/dist/components/button/button.js';
|
||||
import '@components/page-header/page-header';
|
||||
import type { PageAction } from '@components/page-header/page-header';
|
||||
import { designTokens } from '../../styles/tokens.css';
|
||||
import { downloadStore, stateLabel } from '@store/download-store';
|
||||
import type { Request, RequestSummary, DownloadView as DownloadRecord } from '@store/download-store';
|
||||
@@ -246,23 +247,23 @@ export class DownloadsView extends ViewLifecycleMixin(LitElement) {
|
||||
|
||||
override render() {
|
||||
return html`
|
||||
<page-header heading="Downloads">
|
||||
${this.tab === 'requests'
|
||||
? html`
|
||||
<wa-button
|
||||
slot="actions"
|
||||
size="small"
|
||||
appearance="outlined"
|
||||
?disabled=${this.checking}
|
||||
title="Search every download client for everything on this list right now, instead of waiting for the next scheduled check"
|
||||
@click=${() => void this.checkNow()}
|
||||
>
|
||||
<wa-icon slot="start" name="rotate"></wa-icon>
|
||||
${this.checking ? 'Searching…' : 'Check now'}
|
||||
</wa-button>
|
||||
`
|
||||
: nothing}
|
||||
</page-header>
|
||||
<page-header
|
||||
heading="Downloads"
|
||||
.actions=${this.tab === 'requests'
|
||||
? ([
|
||||
{
|
||||
id: 'check-now',
|
||||
label: this.checking
|
||||
? 'Searching\u2026'
|
||||
: 'Check now',
|
||||
icon: 'rotate',
|
||||
disabled: this.checking,
|
||||
title: 'Search every download client for everything on this list right now, instead of waiting for the next scheduled check',
|
||||
onSelect: () => void this.checkNow(),
|
||||
},
|
||||
] satisfies PageAction[])
|
||||
: []}
|
||||
></page-header>
|
||||
|
||||
<p class="subtitle">
|
||||
Music you have requested, and the download attempts that
|
||||
|
||||
@@ -1,8 +1,9 @@
|
||||
import { LitElement, html, css, nothing } from 'lit';
|
||||
import { customElement, state } from 'lit/decorators.js';
|
||||
import '@awesome.me/webawesome/dist/components/icon/icon.js';
|
||||
import '@awesome.me/webawesome/dist/components/button/button.js';
|
||||
import { GetShelves } from '@go/home/service.js';
|
||||
import { ICON_SHUFFLE } from '@utils/icon-language';
|
||||
import type { PageAction } from '@components/page-header/page-header';
|
||||
import { GetAlbumTracks } from '@go/library/library.js';
|
||||
import type * as home from '@go/home/models.js';
|
||||
import type * as library from '@go/library/models.js';
|
||||
@@ -160,29 +161,52 @@ export class HomeView extends ViewLifecycleMixin(LitElement) {
|
||||
user-select: none;
|
||||
}
|
||||
|
||||
/*
|
||||
* The hover play button is a *hover* affordance, so it is
|
||||
* gated on the device having hover rather than on width. A
|
||||
* touch long-press synthesises a hover state in the WebView,
|
||||
* so on a phone it flashed into view during the 500ms hold
|
||||
* that utils/long-press.ts is measuring for a context menu —
|
||||
* a control appearing because you were reaching for a
|
||||
* different one. A phone user taps the album and plays from
|
||||
* the detail view, so there is nothing to replace it with.
|
||||
*
|
||||
* display:none outside the query rather than opacity:0 on
|
||||
* its own: an opacity-0 button still takes taps and is
|
||||
* still in the accessibility tree, so the invisible control
|
||||
* would keep the hit area it was never meant to have on
|
||||
* touch. Everything else stays inside, so the desktop
|
||||
* animation is unchanged.
|
||||
*/
|
||||
.play {
|
||||
position: absolute;
|
||||
right: 8px;
|
||||
bottom: 8px;
|
||||
width: 38px;
|
||||
height: 38px;
|
||||
border: none;
|
||||
border-radius: 50%;
|
||||
background: var(--yj-accent, #ffd43b);
|
||||
color: var(--yj-accent-fg, #000);
|
||||
display: flex;
|
||||
align-items: center;
|
||||
justify-content: center;
|
||||
cursor: pointer;
|
||||
opacity: 0;
|
||||
transform: translateY(6px);
|
||||
transition: opacity 0.12s ease, transform 0.12s ease;
|
||||
display: none;
|
||||
}
|
||||
|
||||
.card:hover .play,
|
||||
.card:focus-within .play {
|
||||
opacity: 1;
|
||||
transform: translateY(0);
|
||||
@media (hover: hover) and (pointer: fine) {
|
||||
.play {
|
||||
position: absolute;
|
||||
right: 8px;
|
||||
bottom: 8px;
|
||||
width: 38px;
|
||||
height: 38px;
|
||||
border: none;
|
||||
border-radius: 50%;
|
||||
background: var(--yj-accent, #ffd43b);
|
||||
color: var(--yj-accent-fg, #000);
|
||||
display: flex;
|
||||
align-items: center;
|
||||
justify-content: center;
|
||||
cursor: pointer;
|
||||
opacity: 0;
|
||||
transform: translateY(6px);
|
||||
transition: opacity 0.12s ease, transform 0.12s ease;
|
||||
}
|
||||
|
||||
.card:hover .play,
|
||||
.card:focus-within .play {
|
||||
opacity: 1;
|
||||
transform: translateY(0);
|
||||
}
|
||||
}
|
||||
|
||||
.name {
|
||||
@@ -260,23 +284,24 @@ export class HomeView extends ViewLifecycleMixin(LitElement) {
|
||||
|
||||
override render() {
|
||||
return html`
|
||||
<page-header heading="Home">
|
||||
<!-- "Shuffle" alone was two different controls with one
|
||||
name: this one and the transport's shuffle mode.
|
||||
They were never on screen together until the app
|
||||
started landing on Home (H-8), and a cached view is
|
||||
in the accessibility tree either way. -->
|
||||
<wa-button
|
||||
slot="actions"
|
||||
size="small"
|
||||
appearance="plain"
|
||||
title="Reshuffle the suggestions"
|
||||
@click=${() => void this.load()}
|
||||
>
|
||||
<wa-icon slot="start" name="shuffle"></wa-icon>
|
||||
Shuffle suggestions
|
||||
</wa-button>
|
||||
</page-header>
|
||||
<page-header
|
||||
heading="Home"
|
||||
.actions=${[
|
||||
{
|
||||
// "Shuffle" alone was two different controls
|
||||
// with one name: this one and the transport's
|
||||
// shuffle mode. They were never on screen
|
||||
// together until the app started landing on
|
||||
// Home (H-8), and a cached view is in the
|
||||
// accessibility tree either way.
|
||||
id: 'shuffle-suggestions',
|
||||
label: 'Shuffle suggestions',
|
||||
icon: ICON_SHUFFLE,
|
||||
title: 'Reshuffle the suggestions',
|
||||
onSelect: () => void this.load(),
|
||||
},
|
||||
] satisfies PageAction[]}
|
||||
></page-header>
|
||||
<p class="lede">Somewhere to start listening.</p>
|
||||
${this.renderBody()}
|
||||
`;
|
||||
|
||||
@@ -13,6 +13,7 @@ import {
|
||||
isQueueSourceNavigable,
|
||||
navigateToQueueSource,
|
||||
} from '@utils/queue-source-link';
|
||||
import { PHONE_QUERY } from '@utils/breakpoints';
|
||||
import { PlayerController } from '@store/controllers/player-controller';
|
||||
import { creditStore } from '@store/credit-store';
|
||||
import { QueueController } from '@store/controllers/queue-controller';
|
||||
@@ -80,6 +81,19 @@ export class NowPlaying extends LitElement {
|
||||
|
||||
private reduceMotionQuery?: MediaQueryList;
|
||||
|
||||
/**
|
||||
* Phone width, from the shell's own breakpoint.
|
||||
*
|
||||
* This is in JS rather than in the stylesheet because what changes
|
||||
* is the *content*, not its appearance: the title, artist and
|
||||
* source render as plain text instead of as links, and no CSS rule
|
||||
* can take a click handler off an element.
|
||||
*/
|
||||
@state()
|
||||
private phone = false;
|
||||
|
||||
private phoneQuery?: MediaQueryList;
|
||||
|
||||
/** Whether each field is actively mid-scroll (class toggle). */
|
||||
@state()
|
||||
private titleScrolling = false;
|
||||
@@ -341,6 +355,12 @@ export class NowPlaying extends LitElement {
|
||||
this.reduceMotion = this.reduceMotionQuery?.matches ?? false;
|
||||
this.reduceMotionQuery?.addEventListener('change', this.handleReduceMotionChange);
|
||||
|
||||
// Same reasoning as above: looked up here, not at module load,
|
||||
// so a test can install its own matchMedia first.
|
||||
this.phoneQuery = window.matchMedia?.(PHONE_QUERY);
|
||||
this.phone = this.phoneQuery?.matches ?? false;
|
||||
this.phoneQuery?.addEventListener('change', this.handlePhoneChange);
|
||||
|
||||
this.resizeObserver = new ResizeObserver(() => {
|
||||
this.geometryDirty = true;
|
||||
this.requestUpdate();
|
||||
@@ -364,6 +384,7 @@ export class NowPlaying extends LitElement {
|
||||
this.attachDragListeners(false);
|
||||
window.removeEventListener(SCROLL_CHANGE_EVENT, this.handleScrollModeEvent);
|
||||
this.reduceMotionQuery?.removeEventListener('change', this.handleReduceMotionChange);
|
||||
this.phoneQuery?.removeEventListener('change', this.handlePhoneChange);
|
||||
this.resizeObserver?.disconnect();
|
||||
this.stopScrollCycle('title');
|
||||
this.stopScrollCycle('artist');
|
||||
@@ -488,7 +509,7 @@ export class NowPlaying extends LitElement {
|
||||
@mouseleave=${this.handleTitleMouseLeave}
|
||||
@transitionend=${() => this.onScrollCycleEnd('title')}
|
||||
>
|
||||
<span class="scroll-content">${trackLink(track.title, track.album, track.releaseGroupMbid, track.recordingMbid) || track.title}</span>
|
||||
<span class="scroll-content">${this.phone ? track.title : trackLink(track.title, track.album, track.releaseGroupMbid, track.recordingMbid) || track.title}</span>
|
||||
</span>
|
||||
<span
|
||||
class="track-artist ${artistScrolling ? 'will-scroll' : ''} ${this.artistScrolling ? 'scrolling' : ''}"
|
||||
@@ -498,14 +519,15 @@ export class NowPlaying extends LitElement {
|
||||
@mouseleave=${this.handleArtistMouseLeave}
|
||||
@transitionend=${() => this.onScrollCycleEnd('artist')}
|
||||
>
|
||||
<span class="scroll-content">${creditLink(creditStore.credits(track.recordingMbid), track.artist, track.artistMbid) || 'Unknown Artist'}</span>
|
||||
<span class="scroll-content">${this.phone ? track.artist || 'Unknown Artist' : creditLink(creditStore.credits(track.recordingMbid), track.artist, track.artistMbid) || 'Unknown Artist'}</span>
|
||||
</span>
|
||||
${describeQueueSource(this.queue.source)
|
||||
? html`
|
||||
<span
|
||||
class="track-source ${isQueueSourceNavigable(this.queue.source) ? 'navigable' : ''}"
|
||||
class="track-source ${!this.phone && isQueueSourceNavigable(this.queue.source) ? 'navigable' : ''}"
|
||||
data-testid="now-playing-source"
|
||||
@click=${(e: MouseEvent) => {
|
||||
if (this.phone) return;
|
||||
if (!isQueueSourceNavigable(this.queue.source)) return;
|
||||
navigateToQueueSource(
|
||||
e.currentTarget as EventTarget,
|
||||
@@ -571,6 +593,10 @@ export class NowPlaying extends LitElement {
|
||||
this.reduceMotion = e.matches;
|
||||
};
|
||||
|
||||
private handlePhoneChange = (e: MediaQueryListEvent): void => {
|
||||
this.phone = e.matches;
|
||||
};
|
||||
|
||||
private shouldScroll(field: 'title' | 'artist'): boolean {
|
||||
const overflows = field === 'title' ? this.titleOverflows : this.artistOverflows;
|
||||
|
||||
@@ -606,6 +632,12 @@ export class NowPlaying extends LitElement {
|
||||
track?.artist ?? '',
|
||||
this.shouldScroll('title') ? '1' : '0',
|
||||
this.shouldScroll('artist') ? '1' : '0',
|
||||
// Crossing the breakpoint swaps a link for a bare string,
|
||||
// and a link is not guaranteed to measure the same as the
|
||||
// text inside it. The marquee travels a distance read from
|
||||
// that measurement, so this belongs in the key even though
|
||||
// the words are identical either side.
|
||||
this.phone ? '1' : '0',
|
||||
].join('\u0000');
|
||||
}
|
||||
|
||||
|
||||
@@ -1,8 +1,16 @@
|
||||
import { LitElement, html, css, nothing } from 'lit';
|
||||
import { customElement, property } from 'lit/decorators.js';
|
||||
import { customElement, property, query, state } from 'lit/decorators.js';
|
||||
import '@awesome.me/webawesome/dist/components/icon/icon.js';
|
||||
import '@awesome.me/webawesome/dist/components/popup/popup.js';
|
||||
import '@awesome.me/webawesome/dist/components/dropdown-item/dropdown-item.js';
|
||||
import type WaPopup from '@awesome.me/webawesome/dist/components/popup/popup.js';
|
||||
|
||||
import { designTokens } from '../../styles/tokens.css';
|
||||
import {
|
||||
MenuKeyboard,
|
||||
contextMenuStyles,
|
||||
} from '../../utils/context-menu-controller';
|
||||
import { ICON_MORE_ACTIONS } from '../../utils/icon-language';
|
||||
|
||||
/**
|
||||
* The one arrangement every primary view uses to say what it is.
|
||||
@@ -18,6 +26,19 @@ import { designTokens } from '../../styles/tokens.css';
|
||||
* Title, count, sort, actions — in that order, in one component, so a
|
||||
* new view gets the shape by using it rather than by copying whichever
|
||||
* neighbour it happened to read.
|
||||
*
|
||||
* **Actions are data, and `<slot name="actions">` is the exception.**
|
||||
* Playlists' three buttons totalled 390px inside a header that gets
|
||||
* 700px at 900×600 and clipped "New Smart Playlist" to 114 of its 162
|
||||
* (#69) — a live defect at a size the app promises, against plan 018's
|
||||
* *no action is ever unreachable at any supported size*. The header
|
||||
* cannot fix that for slotted markup: it cannot move another
|
||||
* component's light-DOM children into a dropdown and keep their
|
||||
* behaviour, and arbitrary markup offers nothing generic to render as
|
||||
* a menu item. So a host declares `PageAction[]` and the header picks
|
||||
* the rendering. The slot survives for markup a data list genuinely
|
||||
* cannot express, at the stated cost that **a slotted action does not
|
||||
* collapse** and must therefore fit at 800×600.
|
||||
*/
|
||||
|
||||
export interface SortOption {
|
||||
@@ -27,6 +48,50 @@ export interface SortOption {
|
||||
|
||||
export type SortDirection = 'asc' | 'desc';
|
||||
|
||||
/**
|
||||
* An action that only makes sense while it is a button.
|
||||
*
|
||||
* A drop target is the case: you cannot drag a track onto a closed
|
||||
* menu, so the affordance is absent from the overflow rather than
|
||||
* approximated there. The header wires these onto the button it
|
||||
* renders and owns none of them — the same division the sort control
|
||||
* already lives by.
|
||||
*/
|
||||
export interface PageActionDrop {
|
||||
/** True while an acceptable payload is over the button. */
|
||||
active?: boolean;
|
||||
onDragOver: (e: DragEvent) => void;
|
||||
onDragLeave: (e: DragEvent) => void;
|
||||
onDrop: (e: DragEvent) => void;
|
||||
}
|
||||
|
||||
/**
|
||||
* One thing a view can do, as data rather than as markup.
|
||||
*
|
||||
* `<slot name="actions">` cannot be collapsed, and that is a fact about
|
||||
* the API rather than an effort estimate (#69): a component cannot move
|
||||
* another component's light-DOM children into a dropdown and keep their
|
||||
* behaviour, and there is nothing generic in arbitrary markup to render
|
||||
* as a menu item. Declaring an action instead is what lets the header
|
||||
* choose between the two renderings.
|
||||
*/
|
||||
export interface PageAction {
|
||||
id: string;
|
||||
label: string;
|
||||
/** From `utils/icon-language`, never a literal. */
|
||||
icon: string;
|
||||
onSelect: () => void;
|
||||
/**
|
||||
* Higher survives longer. The lowest collapses first, ties broken
|
||||
* by declaration order from the right, so a host that says nothing
|
||||
* gets "the last one written goes first".
|
||||
*/
|
||||
priority?: number;
|
||||
disabled?: boolean;
|
||||
title?: string;
|
||||
drop?: PageActionDrop;
|
||||
}
|
||||
|
||||
@customElement('page-header')
|
||||
export class PageHeader extends LitElement {
|
||||
/**
|
||||
@@ -80,8 +145,82 @@ export class PageHeader extends LitElement {
|
||||
@property({ type: Boolean })
|
||||
busy = false;
|
||||
|
||||
/**
|
||||
* What this view can do, in the order it wants them shown.
|
||||
*
|
||||
* The header decides what *fits*; the host decides what *happens*.
|
||||
* That is the rule the sort control already lives by — it asks for
|
||||
* a sort rather than performing one — and actions follow it, which
|
||||
* is why an action carries a handler rather than the header
|
||||
* carrying a verb it would have to interpret.
|
||||
*/
|
||||
@property({ attribute: false })
|
||||
actions: PageAction[] = [];
|
||||
|
||||
/** Action ids currently in the overflow menu. Derived, never set by a host. */
|
||||
@state()
|
||||
private collapsed: ReadonlySet<string> = new Set();
|
||||
|
||||
@state()
|
||||
private menuOpen = false;
|
||||
|
||||
@query('.page-header')
|
||||
private headerEl?: HTMLElement;
|
||||
|
||||
@query('.more-button')
|
||||
private moreButton?: HTMLButtonElement;
|
||||
|
||||
@query('#page-header-overflow')
|
||||
private menuPanel?: HTMLElement;
|
||||
|
||||
@query('wa-popup')
|
||||
private popup?: WaPopup;
|
||||
|
||||
private menuKeyboard = new MenuKeyboard(() => this.closeMenu());
|
||||
|
||||
private resizeObserver?: ResizeObserver;
|
||||
|
||||
/**
|
||||
* Whether the outside-click listener is attached.
|
||||
*
|
||||
* A `removeEventListener` with no matching `add` is not harmless
|
||||
* here: `view-lifecycle.test.ts` counts document listeners across a
|
||||
* view's life and an unconditional detach on disconnect shows up as
|
||||
* `held: -1`, which is the same accounting that would hide a real
|
||||
* leak in the other direction.
|
||||
*/
|
||||
private outsideCloseAttached = false;
|
||||
|
||||
/**
|
||||
* What the last fit was measured against.
|
||||
*
|
||||
* `updated()` runs on every pass, so it has to say what it depends
|
||||
* on or it re-measures — and a measurement here forces synchronous
|
||||
* layout. Width changes arrive through the ResizeObserver; this key
|
||||
* covers everything *else* in the flex row that can change how much
|
||||
* of it the actions are left.
|
||||
|
||||
*/
|
||||
private lastFitKey = '';
|
||||
|
||||
override connectedCallback(): void {
|
||||
super.connectedCallback();
|
||||
|
||||
this.resizeObserver = new ResizeObserver(() => this.measureFit());
|
||||
this.resizeObserver.observe(this);
|
||||
}
|
||||
|
||||
override disconnectedCallback(): void {
|
||||
super.disconnectedCallback();
|
||||
|
||||
this.resizeObserver?.disconnect();
|
||||
this.resizeObserver = undefined;
|
||||
this.detachOutsideClose();
|
||||
}
|
||||
|
||||
static override styles = [
|
||||
designTokens,
|
||||
contextMenuStyles,
|
||||
css`
|
||||
:host {
|
||||
display: block;
|
||||
@@ -96,12 +235,26 @@ export class PageHeader extends LitElement {
|
||||
border-bottom: 1px solid var(--yj-border-subtle, #333);
|
||||
}
|
||||
|
||||
/* The title gives way before an action does.
|
||||
|
||||
Everything in this row was flex-shrink: 0, so whatever
|
||||
came last lost — and the actions come last, which is how
|
||||
the "More actions" button ended up 76px off the right
|
||||
edge of a 320px viewport with every action already
|
||||
collapsed into it. The title is the one thing here the
|
||||
navigation also says (the sidebar item is selected, the
|
||||
bottom-nav tab is current), so it is the cheapest thing
|
||||
to truncate; the count, the sort and the actions are each
|
||||
the only place they are said. */
|
||||
h1 {
|
||||
margin: 0;
|
||||
font-size: var(--yj-text-xl, 18px);
|
||||
font-weight: 600;
|
||||
color: var(--yj-text-primary, #fff);
|
||||
white-space: nowrap;
|
||||
overflow: hidden;
|
||||
text-overflow: ellipsis;
|
||||
min-width: 0;
|
||||
}
|
||||
|
||||
.count {
|
||||
@@ -191,6 +344,98 @@ export class PageHeader extends LitElement {
|
||||
::slotted(*) {
|
||||
flex-shrink: 0;
|
||||
}
|
||||
|
||||
.actions {
|
||||
display: flex;
|
||||
align-items: center;
|
||||
gap: 8px;
|
||||
flex-shrink: 0;
|
||||
}
|
||||
|
||||
.action,
|
||||
.more-button {
|
||||
background: none;
|
||||
border: 1px solid var(--yj-border-subtle, #555);
|
||||
border-radius: 4px;
|
||||
color: var(--yj-text-primary, #fff);
|
||||
padding: 6px 12px;
|
||||
font-size: var(--yj-text-md, 13px);
|
||||
font-family: inherit;
|
||||
cursor: pointer;
|
||||
display: flex;
|
||||
align-items: center;
|
||||
gap: 6px;
|
||||
white-space: nowrap;
|
||||
flex-shrink: 0;
|
||||
}
|
||||
|
||||
.more-button {
|
||||
padding: 6px 10px;
|
||||
}
|
||||
|
||||
/* The display: flex above outranks the UA stylesheet's
|
||||
rule for [hidden], and hiding is how an action
|
||||
collapses. (No backticks in here: one ends the css
|
||||
literal, and what you get is "css(...) is not a
|
||||
function" a long way from the cause.) */
|
||||
.action[hidden],
|
||||
.more-button[hidden] {
|
||||
display: none;
|
||||
}
|
||||
|
||||
.action:hover,
|
||||
.more-button:hover,
|
||||
.action.drag-over {
|
||||
border-color: var(--yj-accent, #ffd43b);
|
||||
color: var(--yj-accent-text, #ffd43b);
|
||||
}
|
||||
|
||||
.action.drag-over {
|
||||
background-color: var(
|
||||
--yj-accent-bg-strong,
|
||||
rgba(255, 212, 59, 0.15)
|
||||
);
|
||||
}
|
||||
|
||||
.action:disabled {
|
||||
opacity: 0.5;
|
||||
cursor: default;
|
||||
}
|
||||
|
||||
.action:focus-visible,
|
||||
.more-button:focus-visible {
|
||||
outline: 2px solid var(--yj-accent, #ffd43b);
|
||||
outline-offset: -1px;
|
||||
}
|
||||
|
||||
wa-popup {
|
||||
z-index: 200;
|
||||
}
|
||||
|
||||
/* A component states what it drops at phone width itself,
|
||||
in its own stylesheet, because a media query inside a
|
||||
shadow root is answered by the viewport and the shell
|
||||
cannot reach in. Here that is one word: the sort control
|
||||
is 172px of a 320px header, and "Sort:" is ~40px of it
|
||||
for a label the adjacent direction arrow already implies.
|
||||
It stays in the accessibility tree — it is the select's
|
||||
accessible name, so hiding it outright would rename the
|
||||
control to nothing — which is config-field's bug, one
|
||||
component over. clip-path rather than display: none for
|
||||
the reason styles/sr-only.css.ts gives. */
|
||||
@media (max-width: 599px) {
|
||||
.sort-label {
|
||||
position: absolute;
|
||||
width: 1px;
|
||||
height: 1px;
|
||||
margin: -1px;
|
||||
padding: 0;
|
||||
overflow: hidden;
|
||||
clip-path: inset(50%);
|
||||
white-space: nowrap;
|
||||
border: 0;
|
||||
}
|
||||
}
|
||||
`,
|
||||
];
|
||||
|
||||
@@ -212,11 +457,293 @@ export class PageHeader extends LitElement {
|
||||
${this.renderCount()}
|
||||
<div class="spacer"></div>
|
||||
${this.renderScope()} ${this.renderSort()}
|
||||
${this.renderActions()}
|
||||
<slot name="actions"></slot>
|
||||
</header>
|
||||
`;
|
||||
}
|
||||
|
||||
protected override updated(): void {
|
||||
const key = [
|
||||
this.heading,
|
||||
this.count,
|
||||
this.countNoun,
|
||||
this.countPlural,
|
||||
this.searchTerm,
|
||||
this.sortOptions.length,
|
||||
this.sortField,
|
||||
this.sortDirection,
|
||||
this.busy,
|
||||
this.actions.map((a) => `${a.id}:${a.label}:${a.disabled ?? false}`).join(','),
|
||||
].join('|');
|
||||
|
||||
if (key === this.lastFitKey) return;
|
||||
|
||||
this.lastFitKey = key;
|
||||
this.measureFit();
|
||||
}
|
||||
|
||||
// =================================================================
|
||||
// What fits
|
||||
// =================================================================
|
||||
|
||||
/**
|
||||
* Decide which actions are buttons and which are menu items.
|
||||
*
|
||||
* Two things about the shape of this are load-bearing.
|
||||
*
|
||||
* **Every pass starts from all-visible**, so the collapsed set is a
|
||||
* pure function of the current width rather than of the order the
|
||||
* widths arrived in. A rule that only ever *added* to the set would
|
||||
* never give an action back when the window grew, and one that
|
||||
* adjusted by a step would need a hysteresis band to stop it
|
||||
* oscillating on the pixel where a button exactly fits.
|
||||
*
|
||||
* **It flips `hidden` on the rendered nodes rather than re-rendering
|
||||
* between steps.** Reading `scrollWidth` forces layout, which is the
|
||||
* point; awaiting a Lit update between steps instead would let the
|
||||
* intermediate all-visible state paint, so the fix would flash the
|
||||
* overflow it exists to prevent. The reactive state is set once, at
|
||||
* the end, and the next render agrees with what was measured.
|
||||
*
|
||||
* The budget is the *header's* overflow and not the actions row's,
|
||||
* because the count and the sort control are `flex-shrink: 0` and
|
||||
* are therefore competing for the same width — only `.scope` gives
|
||||
* way, which is what it has an ellipsis for.
|
||||
*/
|
||||
private measureFit(): void {
|
||||
const header = this.headerEl;
|
||||
|
||||
if (!header) return;
|
||||
|
||||
if (this.actions.length === 0) {
|
||||
this.commitCollapsed(new Set());
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
const buttons = new Map<string, HTMLElement>();
|
||||
|
||||
for (const el of this.renderRoot.querySelectorAll<HTMLElement>(
|
||||
'[data-action-id]',
|
||||
)) {
|
||||
const id = el.dataset['actionId'];
|
||||
|
||||
if (id !== undefined) buttons.set(id, el);
|
||||
}
|
||||
|
||||
const more = this.moreButton;
|
||||
const title = this.renderRoot.querySelector('h1');
|
||||
|
||||
/**
|
||||
* Nothing is clipped — which is not the same as the header not
|
||||
* overflowing, and the difference is a trap worth naming.
|
||||
*
|
||||
* Once the title can ellipsis, it absorbs the pressure and
|
||||
* `scrollWidth` reports a header that fits perfectly while the
|
||||
* heading reads "Playlis…". That is this issue's own failure
|
||||
* mode moved from the button to the title, and it is invisible
|
||||
* to exactly the same measurement that missed it the first time.
|
||||
* So the title's own truncation counts as not fitting, and
|
||||
* collapsing an action is tried before the title gives way.
|
||||
*/
|
||||
const fits = () =>
|
||||
header.scrollWidth <= header.clientWidth &&
|
||||
(title === null || title.scrollWidth <= title.clientWidth + 1);
|
||||
|
||||
for (const el of buttons.values()) el.hidden = false;
|
||||
|
||||
if (more) more.hidden = true;
|
||||
|
||||
const collapsed = new Set<string>();
|
||||
|
||||
if (!fits()) {
|
||||
if (more) more.hidden = false;
|
||||
|
||||
for (const action of this.collapseOrder()) {
|
||||
collapsed.add(action.id);
|
||||
|
||||
const el = buttons.get(action.id);
|
||||
|
||||
if (el) el.hidden = true;
|
||||
|
||||
if (fits()) break;
|
||||
}
|
||||
}
|
||||
|
||||
this.commitCollapsed(collapsed);
|
||||
}
|
||||
|
||||
/** Lowest priority first; ties broken from the right. */
|
||||
private collapseOrder(): PageAction[] {
|
||||
return this.actions
|
||||
.map((action, index) => ({ action, index }))
|
||||
.sort(
|
||||
(a, b) =>
|
||||
(a.action.priority ?? 0) - (b.action.priority ?? 0) ||
|
||||
b.index - a.index,
|
||||
)
|
||||
.map(({ action }) => action);
|
||||
}
|
||||
|
||||
private commitCollapsed(next: Set<string>): void {
|
||||
const same =
|
||||
next.size === this.collapsed.size &&
|
||||
[...next].every((id) => this.collapsed.has(id));
|
||||
|
||||
if (same) return;
|
||||
|
||||
this.collapsed = next;
|
||||
|
||||
// Nothing left to show in it. Closing rather than leaving an
|
||||
// empty menu open is the same rule the shelves follow.
|
||||
if (next.size === 0 && this.menuOpen) this.closeMenu();
|
||||
}
|
||||
|
||||
// =================================================================
|
||||
// Rendering
|
||||
// =================================================================
|
||||
|
||||
private renderActions() {
|
||||
if (this.actions.length === 0) return nothing;
|
||||
|
||||
const overflowed = this.actions.filter((a) => this.collapsed.has(a.id));
|
||||
|
||||
return html`
|
||||
<div class="actions">
|
||||
${this.actions.map((a) => this.renderActionButton(a))}
|
||||
<wa-popup
|
||||
placement="bottom-end"
|
||||
flip
|
||||
shift
|
||||
.active=${this.menuOpen}
|
||||
>
|
||||
<button
|
||||
slot="anchor"
|
||||
class="more-button"
|
||||
type="button"
|
||||
data-testid="page-actions-more"
|
||||
aria-label="More actions"
|
||||
aria-haspopup="menu"
|
||||
aria-expanded=${this.menuOpen ? 'true' : 'false'}
|
||||
aria-controls="page-header-overflow"
|
||||
?hidden=${overflowed.length === 0}
|
||||
@click=${this.onMoreClick}
|
||||
>
|
||||
<wa-icon name=${ICON_MORE_ACTIONS}></wa-icon>
|
||||
</button>
|
||||
<div
|
||||
id="page-header-overflow"
|
||||
class="context-menu-panel"
|
||||
role="menu"
|
||||
aria-label="More actions"
|
||||
>
|
||||
${overflowed.map(
|
||||
(a) => html`
|
||||
<wa-dropdown-item
|
||||
?disabled=${a.disabled ?? false}
|
||||
@click=${() => this.onActionSelect(a)}
|
||||
>
|
||||
<wa-icon
|
||||
slot="icon"
|
||||
name=${a.icon}
|
||||
></wa-icon>
|
||||
${a.label}
|
||||
</wa-dropdown-item>
|
||||
`,
|
||||
)}
|
||||
</div>
|
||||
</wa-popup>
|
||||
</div>
|
||||
`;
|
||||
}
|
||||
|
||||
private renderActionButton(a: PageAction) {
|
||||
const drop = a.drop;
|
||||
|
||||
return html`
|
||||
<button
|
||||
class="action ${drop?.active === true ? 'drag-over' : ''}"
|
||||
type="button"
|
||||
data-action-id=${a.id}
|
||||
data-testid=${`page-action-${a.id}`}
|
||||
title=${a.title ?? nothing}
|
||||
?disabled=${a.disabled ?? false}
|
||||
?hidden=${this.collapsed.has(a.id)}
|
||||
@click=${() => a.onSelect()}
|
||||
@dragover=${(e: DragEvent) => drop?.onDragOver(e)}
|
||||
@dragleave=${(e: DragEvent) => drop?.onDragLeave(e)}
|
||||
@drop=${(e: DragEvent) => drop?.onDrop(e)}
|
||||
>
|
||||
<wa-icon name=${a.icon}></wa-icon>
|
||||
${a.label}
|
||||
</button>
|
||||
`;
|
||||
}
|
||||
|
||||
// =================================================================
|
||||
// The overflow menu
|
||||
// =================================================================
|
||||
|
||||
private onActionSelect(a: PageAction): void {
|
||||
if (a.disabled === true) return;
|
||||
|
||||
this.closeMenu();
|
||||
a.onSelect();
|
||||
}
|
||||
|
||||
private onMoreClick = (): void => {
|
||||
if (this.menuOpen) {
|
||||
this.closeMenu();
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
this.menuOpen = true;
|
||||
|
||||
void this.updateComplete.then(() => {
|
||||
if (!this.menuOpen) return;
|
||||
|
||||
this.popup?.reposition();
|
||||
this.menuKeyboard.open(this.menuPanel ?? null, this.moreButton);
|
||||
this.attachOutsideClose();
|
||||
});
|
||||
};
|
||||
|
||||
private closeMenu(): void {
|
||||
if (!this.menuOpen) return;
|
||||
|
||||
this.detachOutsideClose();
|
||||
this.menuKeyboard.close();
|
||||
this.menuOpen = false;
|
||||
}
|
||||
|
||||
/**
|
||||
* A click anywhere else closes it. `composedPath` rather than
|
||||
* `contains`, because the trigger and the panel are both inside
|
||||
* this shadow root and a click retargets at the host.
|
||||
*/
|
||||
private onOutsideDown = (e: Event): void => {
|
||||
if (e.composedPath().includes(this.menuPanel as EventTarget)) return;
|
||||
if (e.composedPath().includes(this.moreButton as EventTarget)) return;
|
||||
|
||||
this.closeMenu();
|
||||
};
|
||||
|
||||
private attachOutsideClose(): void {
|
||||
if (this.outsideCloseAttached) return;
|
||||
|
||||
this.outsideCloseAttached = true;
|
||||
document.addEventListener('mousedown', this.onOutsideDown, true);
|
||||
}
|
||||
|
||||
private detachOutsideClose(): void {
|
||||
if (!this.outsideCloseAttached) return;
|
||||
|
||||
this.outsideCloseAttached = false;
|
||||
document.removeEventListener('mousedown', this.onOutsideDown, true);
|
||||
}
|
||||
|
||||
private renderCount() {
|
||||
if (this.count === null) return nothing;
|
||||
|
||||
@@ -258,7 +785,10 @@ export class PageHeader extends LitElement {
|
||||
if (this.sortOptions.length === 1) {
|
||||
return html`
|
||||
<div class="sort">
|
||||
<span>Sort: ${this.sortOptions[0]?.label}</span>
|
||||
<span
|
||||
><span class="sort-label">Sort: </span
|
||||
>${this.sortOptions[0]?.label}</span
|
||||
>
|
||||
${this.renderDirectionButton(ascending)}
|
||||
</div>
|
||||
`;
|
||||
@@ -267,7 +797,7 @@ export class PageHeader extends LitElement {
|
||||
return html`
|
||||
<div class="sort">
|
||||
<label>
|
||||
Sort:
|
||||
<span class="sort-label">Sort:</span>
|
||||
<select
|
||||
data-testid="page-sort"
|
||||
.value=${this.sortField}
|
||||
|
||||
@@ -40,7 +40,10 @@ import type { DuplicateTracksDialog } from '@components/duplicate-tracks-dialog/
|
||||
import {
|
||||
ICON_NEW,
|
||||
ICON_PLAYLIST,
|
||||
ICON_SMART_PLAYLIST,
|
||||
} from '@utils/icon-language';
|
||||
import '@components/page-header/page-header';
|
||||
import type { PageAction } from '@components/page-header/page-header';
|
||||
|
||||
const SCROLL_DEBOUNCE_MS = 100;
|
||||
|
||||
@@ -194,11 +197,6 @@ export class PlaylistView extends ViewLifecycleMixin(LitElement) {
|
||||
contain: layout style;
|
||||
}
|
||||
|
||||
.header-actions {
|
||||
display: flex;
|
||||
gap: 8px;
|
||||
}
|
||||
|
||||
.header-spinner {
|
||||
display: inline-block;
|
||||
width: 14px;
|
||||
@@ -209,33 +207,6 @@ export class PlaylistView extends ViewLifecycleMixin(LitElement) {
|
||||
animation: spin 0.6s linear infinite;
|
||||
}
|
||||
|
||||
.new-playlist-button {
|
||||
background: none;
|
||||
border: 1px solid var(--yj-border-subtle, #555);
|
||||
border-radius: 4px;
|
||||
color: var(--yj-text-primary, #fff);
|
||||
padding: 6px 12px;
|
||||
font-size: 13px;
|
||||
cursor: pointer;
|
||||
display: flex;
|
||||
align-items: center;
|
||||
gap: 6px;
|
||||
font-family: inherit;
|
||||
}
|
||||
|
||||
.new-playlist-button:hover,
|
||||
.new-playlist-button.drag-over {
|
||||
border-color: var(--yj-accent, #ffd43b);
|
||||
color: var(--yj-accent-text, #ffd43b);
|
||||
}
|
||||
|
||||
.new-playlist-button.drag-over {
|
||||
background-color: var(
|
||||
--yj-accent-bg-strong,
|
||||
rgba(255, 212, 59, 0.15)
|
||||
);
|
||||
}
|
||||
|
||||
.create-form {
|
||||
display: flex;
|
||||
align-items: center;
|
||||
@@ -455,25 +426,6 @@ export class PlaylistView extends ViewLifecycleMixin(LitElement) {
|
||||
min-width: 0;
|
||||
}
|
||||
|
||||
.import-button {
|
||||
background: none;
|
||||
border: 1px solid var(--yj-border-subtle, #555);
|
||||
border-radius: 4px;
|
||||
color: var(--yj-text-primary, #fff);
|
||||
padding: 6px 12px;
|
||||
font-size: 13px;
|
||||
cursor: pointer;
|
||||
display: flex;
|
||||
align-items: center;
|
||||
gap: 6px;
|
||||
font-family: inherit;
|
||||
}
|
||||
|
||||
.import-button:hover {
|
||||
border-color: var(--yj-accent, #ffd43b);
|
||||
color: var(--yj-accent-text, #ffd43b);
|
||||
}
|
||||
|
||||
.import-error {
|
||||
padding: 0.5em 0.75em;
|
||||
margin: 0.5em 16px 0;
|
||||
@@ -1022,10 +974,11 @@ export class PlaylistView extends ViewLifecycleMixin(LitElement) {
|
||||
) => {
|
||||
const related =
|
||||
e.relatedTarget as Node | null;
|
||||
const btn =
|
||||
this.shadowRoot?.querySelector(
|
||||
'.new-playlist-button',
|
||||
);
|
||||
// The button the event was bound to, rather than a selector for
|
||||
// it: `page-header` renders it now, so it is not in this shadow
|
||||
// root at all and the old `.new-playlist-button` lookup would
|
||||
// find nothing and leave the highlight stuck on.
|
||||
const btn = e.currentTarget as Element | null;
|
||||
|
||||
if (btn && !btn.contains(related)) {
|
||||
this.dragOverNewButton = false;
|
||||
@@ -1470,6 +1423,49 @@ export class PlaylistView extends ViewLifecycleMixin(LitElement) {
|
||||
this.saveSortPreferences();
|
||||
};
|
||||
|
||||
/**
|
||||
* The three things this page can do, as data.
|
||||
*
|
||||
* The priority order is what #69's Direction asks for and it is
|
||||
* only interesting for one of them: **New Playlist is highest
|
||||
* because it is the drop target**. You cannot drag a track onto a
|
||||
* closed menu, so collapsing it is the one collapse here that
|
||||
* removes a capability rather than relocating it. Import is lowest
|
||||
* because it is the rarest, and at 900×600 it is the only one that
|
||||
* has to go.
|
||||
*/
|
||||
private headerActions(): PageAction[] {
|
||||
return [
|
||||
{
|
||||
id: 'import',
|
||||
label: 'Import',
|
||||
icon: 'file-import',
|
||||
priority: 0,
|
||||
onSelect: () => void this.handleImportPlaylist(),
|
||||
},
|
||||
{
|
||||
id: 'new-playlist',
|
||||
label: 'New Playlist',
|
||||
icon: ICON_NEW,
|
||||
priority: 2,
|
||||
onSelect: () => this.handleNewPlaylistClick(),
|
||||
drop: {
|
||||
active: this.dragOverNewButton,
|
||||
onDragOver: this.onNewButtonDragOver,
|
||||
onDragLeave: this.onNewButtonDragLeave,
|
||||
onDrop: this.onNewButtonDrop,
|
||||
},
|
||||
},
|
||||
{
|
||||
id: 'new-smart-playlist',
|
||||
label: 'New Smart Playlist',
|
||||
icon: ICON_SMART_PLAYLIST,
|
||||
priority: 1,
|
||||
onSelect: () => this.handleNewSmartPlaylistClick(),
|
||||
},
|
||||
];
|
||||
}
|
||||
|
||||
override render() {
|
||||
return html`
|
||||
<page-header
|
||||
@@ -1484,33 +1480,8 @@ export class PlaylistView extends ViewLifecycleMixin(LitElement) {
|
||||
search-term=${this.searchCtrl.term}
|
||||
?busy=${this.refreshing}
|
||||
@sort-change=${this.onPageHeaderSort}
|
||||
.actions=${this.headerActions()}
|
||||
>
|
||||
<div slot="actions" class="header-actions">
|
||||
<button
|
||||
class="import-button"
|
||||
@click=${this.handleImportPlaylist}
|
||||
>
|
||||
<wa-icon name="file-import"></wa-icon>
|
||||
Import
|
||||
</button>
|
||||
<button
|
||||
class="new-playlist-button ${this.dragOverNewButton ? 'drag-over' : ''}"
|
||||
@click=${this.handleNewPlaylistClick}
|
||||
@dragover=${this.onNewButtonDragOver}
|
||||
@dragleave=${this.onNewButtonDragLeave}
|
||||
@drop=${this.onNewButtonDrop}
|
||||
>
|
||||
<wa-icon name=${ICON_NEW}></wa-icon>
|
||||
New Playlist
|
||||
</button>
|
||||
<button
|
||||
class="new-playlist-button"
|
||||
@click=${this.handleNewSmartPlaylistClick}
|
||||
>
|
||||
<wa-icon name="filter"></wa-icon>
|
||||
New Smart Playlist
|
||||
</button>
|
||||
</div>
|
||||
</page-header>
|
||||
|
||||
${this.importError
|
||||
@@ -1765,7 +1736,7 @@ export class PlaylistView extends ViewLifecycleMixin(LitElement) {
|
||||
: entry.summary.IsSmart
|
||||
? html`<wa-icon
|
||||
class="playlist-icon"
|
||||
name="filter"
|
||||
name=${ICON_SMART_PLAYLIST}
|
||||
></wa-icon>`
|
||||
: nothing}
|
||||
${isRenaming
|
||||
|
||||
@@ -72,6 +72,24 @@ const MIN_WIDTH = 200;
|
||||
const MAX_WIDTH = 500;
|
||||
const DEFAULT_WIDTH = 320;
|
||||
|
||||
/**
|
||||
* The narrowest main panel the queue is allowed to leave behind before
|
||||
* it stops being a column and becomes an overlay (plan 018, issue #24).
|
||||
*
|
||||
* There is no cliff to derive this from, and pretending otherwise would
|
||||
* be the more dishonest answer: the track list rescales its columns
|
||||
* continuously (213px down to 124px between main widths of 900 and 544,
|
||||
* with no row overflow at any of them) and the album grid steps 3
|
||||
* columns to 2 without breaking. So this is a judgement, anchored at
|
||||
* both ends — it keeps the *default* 1100px window inline, because an
|
||||
* inline queue is a desktop affordance people choose and demoting the
|
||||
* common case to an overlay would be a regression in feel; and it puts
|
||||
* every case measured as broken on the overlay side, which is 900x600
|
||||
* (main = 379px, where all three of the Playlists header's actions are
|
||||
* clipped) and every phone width (main = 69px at 390, 0px at 320).
|
||||
*/
|
||||
const MAIN_PANEL_FLOOR = 480;
|
||||
|
||||
@customElement('queue-panel')
|
||||
export class QueuePanel
|
||||
extends LitElement
|
||||
@@ -85,6 +103,22 @@ export class QueuePanel
|
||||
@property({ type: Boolean, reflect: true })
|
||||
open = false;
|
||||
|
||||
/**
|
||||
* Whether the panel is covering the content instead of sitting
|
||||
* beside it. **Computed, never set by a caller** — it is reflected
|
||||
* so the stylesheet and a spec can both read it.
|
||||
*
|
||||
* It is deliberately *not* a media query, which is the whole reason
|
||||
* this is a property and not a `@media` block. The panel's width is
|
||||
* user state: drag-resizable between MIN_WIDTH and MAX_WIDTH and
|
||||
* persisted. A breakpoint at a fixed viewport width silently
|
||||
* assumes the default 320, so it is wrong by up to 180px for a user
|
||||
* who has widened the panel — in the direction that hurts, since a
|
||||
* wider queue is exactly when the content can least afford it.
|
||||
*/
|
||||
@property({ type: Boolean, reflect: true })
|
||||
overlay = false;
|
||||
|
||||
@state()
|
||||
private isDragging = false;
|
||||
|
||||
@@ -190,6 +224,19 @@ export class QueuePanel
|
||||
private panelWidth = DEFAULT_WIDTH;
|
||||
private scrollbarDragging = false;
|
||||
|
||||
/** Watches `.content-area`, which is the viewport minus the sidebar. */
|
||||
private spaceObserver?: ResizeObserver;
|
||||
|
||||
/**
|
||||
* What had focus when the overlay opened, so Escape and the scrim
|
||||
* can give it back. Focus is only taken back if the panel had it —
|
||||
* the same rule `MenuKeyboard` follows, for the same reason: the
|
||||
* queue can also be closed by the button in the bottom bar, and
|
||||
* yanking focus away from wherever the user actually is would be
|
||||
* worse than leaving it.
|
||||
*/
|
||||
private overlayOpener: HTMLElement | null = null;
|
||||
|
||||
// _itemSize is an internal property applied via Object.assign in BaseLayout's
|
||||
// config setter. Setting it to match the actual fixed .track-item height (49px)
|
||||
// prevents lit-virtualizer's scroll error correction from fighting the native
|
||||
@@ -265,6 +312,86 @@ export class QueuePanel
|
||||
border-left: 1px solid var(--yj-border-subtle, #333);
|
||||
}
|
||||
|
||||
/* ---------------------------------------------------------
|
||||
Overlay mode (plan 018, #24).
|
||||
|
||||
In flow the panel takes its width *from the main panel*,
|
||||
which is the reported bug: at 900x600 that left 379px and
|
||||
clipped every action in the Playlists header, and at 320px
|
||||
it left 0px — the content was not degraded but gone.
|
||||
|
||||
Here the host spans the whole content area instead and
|
||||
stops being a layout participant, so the main panel keeps
|
||||
its full width and the queue sits over it. The host itself
|
||||
is transparent and click-through; the scrim and the panel
|
||||
are what take pointer events. The containment drops paint,
|
||||
which would otherwise clip the panel's own shadow.
|
||||
--------------------------------------------------------- */
|
||||
:host([overlay]) {
|
||||
position: absolute;
|
||||
inset: 0;
|
||||
width: auto;
|
||||
background-color: transparent;
|
||||
overflow: visible;
|
||||
pointer-events: none;
|
||||
contain: layout style;
|
||||
z-index: 20;
|
||||
}
|
||||
|
||||
/* Closed, an overlay is not there at all. In flow the panel is
|
||||
width: 0, which is its own way of saying this; absolutely
|
||||
positioned there is no width to collapse. */
|
||||
:host([overlay]:not([open])) {
|
||||
display: none;
|
||||
}
|
||||
|
||||
:host([overlay][open]) {
|
||||
border-left: none;
|
||||
}
|
||||
|
||||
:host([overlay]) .panel-content {
|
||||
position: absolute;
|
||||
top: 0;
|
||||
right: 0;
|
||||
bottom: 0;
|
||||
width: var(--queue-width, ${unsafeCSS(DEFAULT_WIDTH)}px);
|
||||
max-width: 100%;
|
||||
box-sizing: border-box;
|
||||
background-color: var(--yj-bg-surface, #212529);
|
||||
border-left: 1px solid var(--yj-border-subtle, #333);
|
||||
box-shadow: -8px 0 24px rgb(0 0 0 / 45%);
|
||||
pointer-events: auto;
|
||||
}
|
||||
|
||||
/* Dragging the edge of something that is already covering the
|
||||
content answers a question nobody asked, and it is a
|
||||
mouse-only affordance either way. */
|
||||
:host([overlay]) .resize-handle {
|
||||
display: none;
|
||||
}
|
||||
|
||||
.scrim {
|
||||
position: absolute;
|
||||
inset: 0;
|
||||
background-color: rgb(0 0 0 / 45%);
|
||||
pointer-events: auto;
|
||||
border: none;
|
||||
padding: 0;
|
||||
margin: 0;
|
||||
cursor: pointer;
|
||||
}
|
||||
|
||||
/* The phone gets the whole width: below 600 there is no
|
||||
"beside" left to be, and this is the shape #55 turns into a
|
||||
real screen. A media query inside a shadow root is answered
|
||||
by the viewport, so the component states this itself rather
|
||||
than the shell reaching in. */
|
||||
@media (max-width: 599px) {
|
||||
:host([overlay]) .panel-content {
|
||||
width: 100%;
|
||||
}
|
||||
}
|
||||
|
||||
.resize-handle {
|
||||
position: absolute;
|
||||
top: 0;
|
||||
@@ -656,6 +783,20 @@ export class QueuePanel
|
||||
'--queue-width',
|
||||
`${this.panelWidth}px`,
|
||||
);
|
||||
|
||||
// The mode is a measurement, so it is observed rather than
|
||||
// computed once: the parent is `.content-area`, whose width is
|
||||
// the viewport minus the sidebar — including the sidebar's own
|
||||
// collapse at 900px, which is what makes 900 the *worst*
|
||||
// desktop width rather than the minimum.
|
||||
this.updateOverlayMode();
|
||||
|
||||
if (this.parentElement) {
|
||||
this.spaceObserver = new ResizeObserver(() =>
|
||||
this.updateOverlayMode(),
|
||||
);
|
||||
this.spaceObserver.observe(this.parentElement);
|
||||
}
|
||||
document.addEventListener(
|
||||
'mousemove',
|
||||
this.handleMouseMove,
|
||||
@@ -690,6 +831,9 @@ export class QueuePanel
|
||||
super.disconnectedCallback();
|
||||
this.creditsUnsub?.();
|
||||
this.creditsUnsub = undefined;
|
||||
this.spaceObserver?.disconnect();
|
||||
this.spaceObserver = undefined;
|
||||
document.removeEventListener('keydown', this.onOverlayKeydown);
|
||||
document.removeEventListener(
|
||||
'mousemove',
|
||||
this.handleMouseMove,
|
||||
@@ -736,7 +880,79 @@ export class QueuePanel
|
||||
this.delegationAttached = false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Decide whether the queue can afford to be a column.
|
||||
*
|
||||
* The parent is `.content-area`, so its width is the viewport minus
|
||||
* the sidebar and the sum already accounts for the sidebar's own
|
||||
* collapse. It is stable across the panel's own open/closed state
|
||||
* in both modes — in flow the panel is a child of that box, and as
|
||||
* an overlay it is out of flow — so this cannot oscillate.
|
||||
*/
|
||||
private updateOverlayMode = () => {
|
||||
const available = this.parentElement?.clientWidth ?? 0;
|
||||
|
||||
// Before layout there is nothing to measure, and answering 0 by
|
||||
// flipping to overlay would show the scrim for a frame.
|
||||
if (available === 0) return;
|
||||
|
||||
this.overlay = available - this.panelWidth < MAIN_PANEL_FLOOR;
|
||||
};
|
||||
|
||||
/**
|
||||
* Escape closes a scrimmed overlay, which is the one keyboard rule
|
||||
* every dialog in this app already follows.
|
||||
*
|
||||
* It is a document listener rather than a panel-scoped binding
|
||||
* (`services/shortcut-scope.ts`) because it is not a *shortcut*: it
|
||||
* is the dismissal of something covering the page, and it has to
|
||||
* work while focus is still behind the scrim. It is attached only
|
||||
* while the overlay is actually up and removed on close, so it is
|
||||
* scoped to a state rather than being a permanent global. Nothing
|
||||
* else binds Escape — the shortcut service only uses it to blur a
|
||||
* text input.
|
||||
*/
|
||||
private onOverlayKeydown = (e: KeyboardEvent) => {
|
||||
if (e.key !== 'Escape' || !this.open || !this.overlay) return;
|
||||
|
||||
e.stopPropagation();
|
||||
this.closeFromOverlay();
|
||||
};
|
||||
|
||||
private closeFromOverlay = () => {
|
||||
const hadFocus = this.contains(
|
||||
document.activeElement as Node | null,
|
||||
);
|
||||
|
||||
this.open = false;
|
||||
|
||||
if (hadFocus) {
|
||||
const back =
|
||||
this.overlayOpener ??
|
||||
document.getElementById('queue-button');
|
||||
|
||||
back?.focus();
|
||||
}
|
||||
|
||||
this.overlayOpener = null;
|
||||
};
|
||||
|
||||
override updated() {
|
||||
// The overlay owns Escape only while it is up.
|
||||
if (this.open && this.overlay) {
|
||||
document.addEventListener('keydown', this.onOverlayKeydown);
|
||||
|
||||
this.overlayOpener ??=
|
||||
document.activeElement instanceof HTMLElement &&
|
||||
document.activeElement !== document.body
|
||||
? document.activeElement
|
||||
: null;
|
||||
} else {
|
||||
document.removeEventListener('keydown', this.onOverlayKeydown);
|
||||
|
||||
if (!this.open) this.overlayOpener = null;
|
||||
}
|
||||
|
||||
// Closed, the panel is `width: 0` — which hides it from the eye
|
||||
// and from nobody else: its Clear and Add buttons still took tab
|
||||
// stops at x=1440 and were still read out (H-5). `inert` is the
|
||||
@@ -1631,6 +1847,11 @@ export class QueuePanel
|
||||
'--queue-width',
|
||||
`${clampedWidth}px`,
|
||||
);
|
||||
|
||||
// Widening the panel is one of the two ways the content can run
|
||||
// out of room, and it is the way a viewport-width media query
|
||||
// cannot see at all.
|
||||
this.updateOverlayMode();
|
||||
};
|
||||
|
||||
private handleMouseUp = () => {
|
||||
@@ -1714,6 +1935,15 @@ export class QueuePanel
|
||||
const tracks = this.queue.tracks;
|
||||
|
||||
return html`
|
||||
${this.overlay
|
||||
? html`<div
|
||||
class="scrim"
|
||||
part="scrim"
|
||||
data-testid="queue-scrim"
|
||||
aria-hidden="true"
|
||||
@click=${this.closeFromOverlay}
|
||||
></div>`
|
||||
: nothing}
|
||||
<div class="panel-content">
|
||||
<div
|
||||
class="resize-handle ${this.isDragging
|
||||
@@ -1763,6 +1993,16 @@ export class QueuePanel
|
||||
name=${ICON_PLAYLIST}
|
||||
></wa-icon>
|
||||
</button>
|
||||
${this.overlay
|
||||
? html`<button
|
||||
class="header-action-button"
|
||||
data-testid="queue-close"
|
||||
aria-label="Close queue"
|
||||
@click=${this.closeFromOverlay}
|
||||
>
|
||||
<wa-icon name="xmark"></wa-icon>
|
||||
</button>`
|
||||
: nothing}
|
||||
</div>
|
||||
</div>
|
||||
|
||||
|
||||
@@ -64,6 +64,7 @@ import { list } from '@utils/binding';
|
||||
import {
|
||||
ICON_PLAYLIST,
|
||||
ICON_QUEUE,
|
||||
ICON_SMART_PLAYLIST,
|
||||
} from '@utils/icon-language';
|
||||
|
||||
|
||||
@@ -1214,7 +1215,7 @@ export class SmartPlaylistDetails
|
||||
<wa-icon name="arrow-left"></wa-icon>
|
||||
</button>
|
||||
<div class="playlist-avatar">
|
||||
<wa-icon name="filter"></wa-icon>
|
||||
<wa-icon name=${ICON_SMART_PLAYLIST}></wa-icon>
|
||||
</div>
|
||||
<div class="playlist-info">
|
||||
<h1
|
||||
|
||||
@@ -11,6 +11,7 @@ import {
|
||||
import { SelectionController } from '@utils/selection-controller';
|
||||
import type { SelectionHost } from '@utils/selection-controller';
|
||||
import { ViewLifecycleMixin } from '@utils/view-lifecycle';
|
||||
import { PHONE_QUERY } from '@utils/breakpoints';
|
||||
import {
|
||||
ContextMenuController,
|
||||
contextMenuStyles,
|
||||
@@ -105,9 +106,6 @@ const ROW_CHROME_WIDTH =
|
||||
const ROW_HEIGHT = 33;
|
||||
const PHONE_ROW_HEIGHT = 52;
|
||||
|
||||
/** The shell's phone breakpoint, as `index.css` and every component
|
||||
* stylesheet spells it. */
|
||||
const PHONE_QUERY = '(max-width: 599px)';
|
||||
|
||||
// Inline SVG paths for favorite icons — eliminates wa-icon shadow DOM
|
||||
// overhead (30-50 shadow roots during scroll). Font Awesome 6 paths.
|
||||
|
||||
@@ -41,6 +41,7 @@ solid/compact-disc
|
||||
solid/copy
|
||||
solid/database
|
||||
solid/download
|
||||
solid/ellipsis
|
||||
solid/file-import
|
||||
solid/filter
|
||||
solid/floppy-disk
|
||||
|
||||
@@ -0,0 +1,23 @@
|
||||
/**
|
||||
* The shell's breakpoints, where JavaScript has to agree with CSS.
|
||||
*
|
||||
* A media query inside a shadow root is answered by the viewport, so a
|
||||
* component normally states what it drops at phone width in its own
|
||||
* stylesheet and needs nothing from here. This exists for the cases
|
||||
* where the decision is not a style: `track-list` computes its grid in
|
||||
* JS from the host width, and `now-playing` renders *different content*
|
||||
* on a phone — a plain string instead of a link — which no stylesheet
|
||||
* can express.
|
||||
*
|
||||
* One breakpoint, several expressions of it. It was a private const in
|
||||
* track-list.ts when there was one; a second reader is where a copy
|
||||
* would start drifting from index.css.
|
||||
*/
|
||||
|
||||
/**
|
||||
* Phone width. 600px rather than the sidebar's 900px because 900 is a
|
||||
* laptop: the answer there is a narrower sidebar, which is still a
|
||||
* sidebar. Below this the shell drops the sidebar column entirely and
|
||||
* bottom-nav takes over.
|
||||
*/
|
||||
export const PHONE_QUERY = '(max-width: 599px)';
|
||||
@@ -54,6 +54,18 @@ export const ICON_PLAYLIST = 'list';
|
||||
*/
|
||||
export const ICON_NEW = 'plus';
|
||||
|
||||
/**
|
||||
* A smart playlist — the rule, and the thing the rule makes.
|
||||
*
|
||||
* Governed for the reason `ICON_AUTOTAG` states: it was already at
|
||||
* three call sites (the Playlists header, the row marker beside a smart
|
||||
* playlist's name, and `smart-playlist-details`'s avatar), and a name
|
||||
* stops being a detail of one component the moment there are two. It is
|
||||
* deliberately *not* `ICON_NEW`, even on the button that makes one:
|
||||
* an icon names the noun it acts on, and the noun here is the rule.
|
||||
*/
|
||||
export const ICON_SMART_PLAYLIST = 'filter';
|
||||
|
||||
/**
|
||||
* The request ("want") toggle, as an outline/solid pair.
|
||||
*
|
||||
@@ -100,6 +112,18 @@ export const ICON_AUTOTAG = 'tag';
|
||||
*/
|
||||
export const ICON_DOWNLOADING = 'download';
|
||||
|
||||
/**
|
||||
* The rest of what this thing can do.
|
||||
*
|
||||
* `page-header` collapses the actions that do not fit into one menu
|
||||
* behind this, so the glyph has to name *more of the same nouns* rather
|
||||
* than any one of them — which is what an ellipsis is and what `bars`
|
||||
* (the navigation drawer, one component over in `bottom-nav`) is not.
|
||||
* It is deliberately the only meaning it carries: an overflow menu that
|
||||
* shared an icon with a destination would be the `list` problem again.
|
||||
*/
|
||||
export const ICON_MORE_ACTIONS = 'ellipsis';
|
||||
|
||||
/**
|
||||
* Take this away.
|
||||
*
|
||||
|
||||
@@ -7,6 +7,7 @@
|
||||
* opening it.
|
||||
*/
|
||||
import { describe, expect, it, beforeEach } from 'vitest';
|
||||
import type { LitElement } from 'lit';
|
||||
|
||||
import '@components/home-view/home-view';
|
||||
import { stub, calls, lastArgs, stubFailure } from '@test/support/harness';
|
||||
@@ -166,7 +167,17 @@ describe('home view', () => {
|
||||
|
||||
const before = calls('home.Service.GetShelves').length;
|
||||
|
||||
shadow<HTMLElement>(el, 'wa-button')!.click();
|
||||
// The action is declared to `page-header` rather than slotted as
|
||||
// markup (#69), so it is a button in *that* shadow root now.
|
||||
const header = shadow<HTMLElement>(el, 'page-header')!;
|
||||
|
||||
await (header as LitElement).updateComplete;
|
||||
|
||||
header.shadowRoot!
|
||||
.querySelector<HTMLButtonElement>(
|
||||
'[data-testid="page-action-shuffle-suggestions"]',
|
||||
)!
|
||||
.click();
|
||||
await el.updateComplete;
|
||||
|
||||
expect(calls('home.Service.GetShelves').length).toBe(before + 1);
|
||||
|
||||
@@ -0,0 +1,85 @@
|
||||
/**
|
||||
* A hover affordance is gated on the device having hover.
|
||||
*
|
||||
* The home page's cover cards reveal a play button on :hover. A touch
|
||||
* long-press synthesises a hover state in the WebView, so on a phone
|
||||
* that button flashed into view during the 500ms hold that
|
||||
* utils/long-press.ts is measuring for a context menu — a control
|
||||
* appearing because the user was reaching for a different one.
|
||||
*
|
||||
* This is asserted against the *parsed stylesheet* rather than by
|
||||
* emulating a touch device, and that is a limitation worth stating
|
||||
* rather than hiding. CDP's Emulation.setEmulatedMedia does not reach
|
||||
* this tier's iframe — matchMedia still answers `hover: hover` after it
|
||||
* is set — so there is no way here to render the component as a phone
|
||||
* would and read the computed style. What can be checked is the shape
|
||||
* the browser actually built from the css`` literal: that the reveal
|
||||
* lives inside a hover media query and that the default is display:none.
|
||||
*
|
||||
* Which is the regression worth catching anyway. The failure mode is
|
||||
* someone hoisting the rule back out of the query for a one-line tidy —
|
||||
* a change nothing renders differently on a desktop, so every other
|
||||
* assertion in this repo passes and the phone silently regresses.
|
||||
*/
|
||||
import { describe, expect, it } from 'vitest';
|
||||
|
||||
import '@components/home-view/home-view';
|
||||
import { fixture } from '@test/support/render';
|
||||
|
||||
/** Every rule in the element's own adopted stylesheets, flattened. */
|
||||
function rulesOf(host: Element): { text: string; condition: string | null }[] {
|
||||
const sheets = host.shadowRoot?.adoptedStyleSheets ?? [];
|
||||
const out: { text: string; condition: string | null }[] = [];
|
||||
|
||||
for (const sheet of sheets) {
|
||||
for (const rule of Array.from(sheet.cssRules)) {
|
||||
if (rule instanceof CSSMediaRule) {
|
||||
for (const inner of Array.from(rule.cssRules)) {
|
||||
out.push({ text: inner.cssText, condition: rule.conditionText });
|
||||
}
|
||||
|
||||
continue;
|
||||
}
|
||||
|
||||
out.push({ text: rule.cssText, condition: null });
|
||||
}
|
||||
}
|
||||
|
||||
return out;
|
||||
}
|
||||
|
||||
describe('the home card play button', () => {
|
||||
it('reveals itself only where the device has hover', async () => {
|
||||
const el = await fixture('home-view', {});
|
||||
const rules = rulesOf(el);
|
||||
|
||||
// The sweep is worth nothing if it read no rules at all — the same
|
||||
// first assertion icon-language.test.ts makes for the same reason.
|
||||
expect(rules.length).toBeGreaterThan(0);
|
||||
|
||||
const reveals = rules.filter(
|
||||
(r) => r.text.includes('.play') && /opacity:\s*1/.test(r.text),
|
||||
);
|
||||
|
||||
expect(reveals.length).toBeGreaterThan(0);
|
||||
|
||||
for (const rule of reveals) {
|
||||
expect(rule.condition).toMatch(/hover:\s*hover/);
|
||||
expect(rule.condition).toMatch(/pointer:\s*fine/);
|
||||
}
|
||||
});
|
||||
|
||||
it('is display:none rather than transparent where it is absent', async () => {
|
||||
const el = await fixture('home-view', {});
|
||||
|
||||
// opacity:0 alone would leave a button that still takes taps and is
|
||||
// still in the accessibility tree, so a phone would keep the hit
|
||||
// area for a control it can never see.
|
||||
const unconditional = rulesOf(el).filter(
|
||||
(r) => r.condition === null && r.text.startsWith('.play'),
|
||||
);
|
||||
|
||||
expect(unconditional.length).toBeGreaterThan(0);
|
||||
expect(unconditional.some((r) => /display:\s*none/.test(r.text))).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -44,6 +44,8 @@ const GOVERNED = [
|
||||
'regular/bookmark',
|
||||
'bars-staggered',
|
||||
'tag',
|
||||
'filter',
|
||||
'ellipsis',
|
||||
];
|
||||
|
||||
/** The one file allowed to say them, plus its own test. */
|
||||
|
||||
@@ -0,0 +1,166 @@
|
||||
/**
|
||||
* The mini player's links are a desktop affordance.
|
||||
*
|
||||
* `utils/explore-link.ts` makes every track and artist name navigate,
|
||||
* and `utils/queue-source-link.ts` makes "Playing from X" navigate — in
|
||||
* the bottom bar those are a few characters of text at a font size
|
||||
* chosen for a bar, which is not a touch target. Worse, explore-link
|
||||
* holds the navigation for one double-click interval and drops it if a
|
||||
* second click arrives: a gesture that exists so double-clicking a row
|
||||
* can play it, and which means nothing at all on touch.
|
||||
*
|
||||
* So below the shell's phone breakpoint the three render as plain text
|
||||
* and the whole bar's cover art opens the full-screen Now Playing view,
|
||||
* which is where the links live.
|
||||
*
|
||||
* The breakpoint is stubbed rather than emulated for the reason
|
||||
* track-list-phone.test.ts states: this tier's viewport is fixed at
|
||||
* 1280x800 by the runner, and the component reads matchMedia in
|
||||
* connectedCallback precisely so a test can answer it first.
|
||||
*/
|
||||
import { describe, expect, it, beforeEach } from 'vitest';
|
||||
|
||||
import '@components/now-playing/now-playing';
|
||||
import { Events } from '../../src/events';
|
||||
import { emit, flush } from '@test/support/harness';
|
||||
import { fixture, shadow, shadowAll, text } from '@test/support/render';
|
||||
import type { TrackInfo } from '@store/player-store';
|
||||
import type { QueueTrack } from '@store/queue-store';
|
||||
|
||||
const TRACK: TrackInfo = {
|
||||
fileName: 'ashes.mp3',
|
||||
filePath: '/music/ashes.mp3',
|
||||
trackLength: 215,
|
||||
seekPosition: 0,
|
||||
state: 'playing',
|
||||
title: 'Ashes to Ashes',
|
||||
artist: 'David Bowie',
|
||||
album: 'Scary Monsters',
|
||||
coverArt: '',
|
||||
coverArtSmall: '',
|
||||
coverArtMedium: '',
|
||||
coverArtLarge: '',
|
||||
trackChangeId: 1,
|
||||
artistMbid: '',
|
||||
releaseGroupMbid: '',
|
||||
recordingMbid: '',
|
||||
};
|
||||
|
||||
function queueTrack(n: number, title: string): QueueTrack {
|
||||
return {
|
||||
id: n,
|
||||
audioFileId: n,
|
||||
filePath: `/music/${n}.mp3`,
|
||||
position: n,
|
||||
title,
|
||||
artist: 'David Bowie',
|
||||
album: 'Scary Monsters',
|
||||
coverArtPath: '',
|
||||
artistMbid: '',
|
||||
releaseGroupMbid: '',
|
||||
recordingMbid: '',
|
||||
};
|
||||
}
|
||||
|
||||
/** Mount the bar with the phone breakpoint answering `matches`. */
|
||||
async function mountAt(phone: boolean) {
|
||||
const real = window.matchMedia.bind(window);
|
||||
|
||||
window.matchMedia = ((q: string) =>
|
||||
q.includes('max-width: 599px')
|
||||
? {
|
||||
matches: phone,
|
||||
media: q,
|
||||
addEventListener() {},
|
||||
removeEventListener() {},
|
||||
}
|
||||
: real(q)) as typeof window.matchMedia;
|
||||
|
||||
try {
|
||||
const el = await fixture('now-playing');
|
||||
|
||||
emit(Events.TrackChanged, { ...TRACK, trackChangeId: 20 });
|
||||
emit(Events.QueueChanged, {
|
||||
tracks: [queueTrack(1, 'Ashes to Ashes')],
|
||||
currentIndex: 0,
|
||||
source: { type: 'album', id: 7, label: 'Scary Monsters' },
|
||||
});
|
||||
await flush();
|
||||
await el.updateComplete;
|
||||
|
||||
return el;
|
||||
} finally {
|
||||
window.matchMedia = real;
|
||||
}
|
||||
}
|
||||
|
||||
describe('the mini player on a phone', () => {
|
||||
beforeEach(() => {
|
||||
emit(Events.TrackChanged, { ...TRACK, trackChangeId: 1 });
|
||||
});
|
||||
|
||||
it('renders the title and artist as plain text', async () => {
|
||||
const el = await mountAt(true);
|
||||
|
||||
expect(shadowAll(el, '.explore-link').length).toBe(0);
|
||||
|
||||
// The words are unchanged — this is about what they are, not about
|
||||
// hiding them. A fix that dropped the text would pass an assertion
|
||||
// about links alone.
|
||||
expect(text(el, '[data-testid="now-playing-title"]')).toContain(
|
||||
'Ashes to Ashes',
|
||||
);
|
||||
expect(text(el, '[data-testid="now-playing-artist"]')).toContain(
|
||||
'David Bowie',
|
||||
);
|
||||
});
|
||||
|
||||
it('does not navigate from the source line', async () => {
|
||||
const el = await mountAt(true);
|
||||
const source = shadow<HTMLElement>(el, '[data-testid="now-playing-source"]');
|
||||
|
||||
expect(source?.classList.contains('navigable')).toBe(false);
|
||||
|
||||
let navigated = false;
|
||||
el.addEventListener('navigate', () => {
|
||||
navigated = true;
|
||||
});
|
||||
|
||||
source?.click();
|
||||
|
||||
expect(navigated).toBe(false);
|
||||
});
|
||||
|
||||
it('still says where the queue came from', async () => {
|
||||
const el = await mountAt(true);
|
||||
|
||||
// Dropping the *link* is the change; dropping the information would
|
||||
// be a different and worse one.
|
||||
expect(text(el, '[data-testid="now-playing-source"]')).toBe(
|
||||
'Playing from Scary Monsters',
|
||||
);
|
||||
});
|
||||
|
||||
it('leaves the desktop bar exactly as it was', async () => {
|
||||
const el = await mountAt(false);
|
||||
|
||||
expect(shadowAll(el, '.explore-link').length).toBeGreaterThan(0);
|
||||
|
||||
const source = shadow<HTMLElement>(el, '[data-testid="now-playing-source"]');
|
||||
|
||||
expect(source?.classList.contains('navigable')).toBe(true);
|
||||
|
||||
let detail: unknown;
|
||||
el.addEventListener('navigate', (e) => {
|
||||
detail = (e as CustomEvent).detail;
|
||||
});
|
||||
|
||||
source?.click();
|
||||
|
||||
expect(detail).toEqual({
|
||||
view: 'explore-album-details',
|
||||
localAlbumId: 7,
|
||||
albumName: 'Scary Monsters',
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -9,7 +9,7 @@
|
||||
* the thing no assertion can — the header looking wrong.
|
||||
*/
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import type { PageHeader } from '@components/page-header/page-header';
|
||||
import type { PageAction, PageHeader } from '@components/page-header/page-header';
|
||||
|
||||
import '@components/page-header/page-header';
|
||||
import { fixture, shadow, shadowAll, update, visual } from '@test/support/render';
|
||||
@@ -19,6 +19,67 @@ const SORTS = [
|
||||
{ id: 'tracks', label: 'Tracks' },
|
||||
];
|
||||
|
||||
/**
|
||||
* Three actions of the shape that broke: Playlists' own, whose widths
|
||||
* (91 + 122 + 162 = 390px) are what a 700px header could not hold.
|
||||
*/
|
||||
function playlistActions(seen: string[]): PageAction[] {
|
||||
return [
|
||||
{
|
||||
id: 'import',
|
||||
label: 'Import',
|
||||
icon: 'file-import',
|
||||
priority: 0,
|
||||
onSelect: () => seen.push('import'),
|
||||
},
|
||||
{
|
||||
id: 'new-playlist',
|
||||
label: 'New Playlist',
|
||||
icon: 'plus',
|
||||
priority: 2,
|
||||
onSelect: () => seen.push('new-playlist'),
|
||||
},
|
||||
{
|
||||
id: 'new-smart-playlist',
|
||||
label: 'New Smart Playlist',
|
||||
icon: 'filter',
|
||||
priority: 1,
|
||||
onSelect: () => seen.push('new-smart-playlist'),
|
||||
},
|
||||
];
|
||||
}
|
||||
|
||||
/**
|
||||
* Resize and let the fit settle.
|
||||
*
|
||||
* The rule is driven by a ResizeObserver, which delivers before paint
|
||||
* and therefore after the microtask queue an `updateComplete` drains —
|
||||
* so this waits on frames rather than on promises, and then on the
|
||||
* render the measurement asks for.
|
||||
*/
|
||||
async function widthOf(el: PageHeader, px: number): Promise<void> {
|
||||
el.style.width = `${px}px`;
|
||||
|
||||
for (let frame = 0; frame < 3; frame += 1) {
|
||||
await new Promise((r) => requestAnimationFrame(r));
|
||||
await el.updateComplete;
|
||||
}
|
||||
}
|
||||
|
||||
/** The labels currently rendered as buttons, in order. */
|
||||
function buttons(el: PageHeader): string[] {
|
||||
return shadowAll<HTMLButtonElement>(el, '.action')
|
||||
.filter((b) => !b.hidden)
|
||||
.map((b) => b.textContent?.trim() ?? '');
|
||||
}
|
||||
|
||||
/** The labels currently in the overflow menu, in order. */
|
||||
function menu(el: PageHeader): string[] {
|
||||
return shadowAll(el, '#page-header-overflow wa-dropdown-item').map(
|
||||
(i) => i.textContent?.trim() ?? '',
|
||||
);
|
||||
}
|
||||
|
||||
describe('<page-header>', () => {
|
||||
it('renders the heading as the page\u2019s only h1', async () => {
|
||||
const el = await fixture<PageHeader>('page-header', {
|
||||
@@ -153,6 +214,264 @@ describe('<page-header>', () => {
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* #69: Playlists slotted three buttons totalling 390px into a header
|
||||
* that gets 700px at 900×600, and "New Smart Playlist" rendered 114 of
|
||||
* its 162. It survived a spec named `layout-overflow` because that one
|
||||
* asserts the *shell* needs no sideways scrolling — clipping inside a
|
||||
* component is invisible to it.
|
||||
*
|
||||
* The header can only fix that for actions it renders itself, which is
|
||||
* why they are data now. These are the assertions about the rule; the
|
||||
* e2e spec is what checks it against the real widths.
|
||||
*/
|
||||
describe('<page-header> actions', () => {
|
||||
it('renders a declared action, and asks the host to perform it', async () => {
|
||||
// Same division the sort control already lives by: the header
|
||||
// decides what fits, the host decides what happens.
|
||||
const seen: string[] = [];
|
||||
const el = await fixture<PageHeader>('page-header', {
|
||||
heading: 'Playlists',
|
||||
actions: playlistActions(seen),
|
||||
});
|
||||
|
||||
await widthOf(el, 1200);
|
||||
|
||||
expect(buttons(el)).toEqual([
|
||||
'Import',
|
||||
'New Playlist',
|
||||
'New Smart Playlist',
|
||||
]);
|
||||
|
||||
shadow<HTMLButtonElement>(el, '[data-testid="page-action-import"]')!.click();
|
||||
|
||||
expect(seen).toEqual(['import']);
|
||||
});
|
||||
|
||||
it('hides the overflow trigger while everything fits', async () => {
|
||||
const el = await fixture<PageHeader>('page-header', {
|
||||
heading: 'Playlists',
|
||||
actions: playlistActions([]),
|
||||
});
|
||||
|
||||
await widthOf(el, 1200);
|
||||
|
||||
expect(shadow<HTMLButtonElement>(el, '.more-button')!.hidden).toBe(true);
|
||||
expect(menu(el)).toEqual([]);
|
||||
});
|
||||
|
||||
it('collapses the lowest priority first', async () => {
|
||||
// Import is lowest because it is rarest; New Playlist is highest
|
||||
// because it is the drop target, and a closed menu cannot be one.
|
||||
const el = await fixture<PageHeader>('page-header', {
|
||||
heading: 'Playlists',
|
||||
count: 4,
|
||||
countNoun: 'playlist',
|
||||
sortOptions: SORTS,
|
||||
sortField: 'name',
|
||||
actions: playlistActions([]),
|
||||
});
|
||||
|
||||
// Asserted as the *order* rather than at two chosen widths: which
|
||||
// pixel drops which button depends on the font and on the shell
|
||||
// this tier does not have, and pinning those numbers here would be
|
||||
// a test of the fixture. What the host declares is a sequence.
|
||||
const states: string[][] = [];
|
||||
|
||||
for (let width = 1200; width >= 300; width -= 40) {
|
||||
await widthOf(el, width);
|
||||
|
||||
const now = menu(el);
|
||||
const last = states[states.length - 1];
|
||||
|
||||
if (last === undefined || last.join() !== now.join()) states.push(now);
|
||||
}
|
||||
|
||||
expect(states).toEqual([
|
||||
[],
|
||||
['Import'],
|
||||
['Import', 'New Smart Playlist'],
|
||||
['Import', 'New Playlist', 'New Smart Playlist'],
|
||||
]);
|
||||
|
||||
// The menu lists them in the host's declared order, not in the
|
||||
// order they happened to collapse — a menu that reshuffles itself
|
||||
// as the window narrows is a menu nobody can learn.
|
||||
expect(buttons(el)).toEqual([]);
|
||||
});
|
||||
|
||||
it('gives an action back when the width returns', async () => {
|
||||
// Every pass starts from all-visible, so the collapsed set is a
|
||||
// function of the current width and not of how it got there. A rule
|
||||
// that only ever added to the set would never widen again.
|
||||
const el = await fixture<PageHeader>('page-header', {
|
||||
heading: 'Playlists',
|
||||
count: 4,
|
||||
countNoun: 'playlist',
|
||||
sortOptions: SORTS,
|
||||
sortField: 'name',
|
||||
actions: playlistActions([]),
|
||||
});
|
||||
|
||||
await widthOf(el, 420);
|
||||
|
||||
expect(buttons(el)).toEqual([]);
|
||||
|
||||
await widthOf(el, 1200);
|
||||
|
||||
expect(menu(el)).toEqual([]);
|
||||
expect(buttons(el)).toEqual([
|
||||
'Import',
|
||||
'New Playlist',
|
||||
'New Smart Playlist',
|
||||
]);
|
||||
});
|
||||
|
||||
it('collapses an action before it truncates the title', async () => {
|
||||
// The title can ellipsis, which means `scrollWidth` reports a
|
||||
// header that fits perfectly while the heading reads "Playlis…" —
|
||||
// this issue's failure mode moved from the button to the title, and
|
||||
// invisible to the same measurement that missed it the first time.
|
||||
const el = await fixture<PageHeader>('page-header', {
|
||||
heading: 'Playlists',
|
||||
count: 4,
|
||||
countNoun: 'playlist',
|
||||
sortOptions: SORTS,
|
||||
sortField: 'name',
|
||||
actions: playlistActions([]),
|
||||
});
|
||||
|
||||
await widthOf(el, 700);
|
||||
|
||||
const h1 = shadow<HTMLElement>(el, 'h1')!;
|
||||
|
||||
expect(h1.scrollWidth).toBeLessThanOrEqual(h1.clientWidth + 1);
|
||||
expect(menu(el).length).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
it('names the overflow trigger and says what it controls', async () => {
|
||||
// An overflow menu is exactly the shape that grows a nameless
|
||||
// control, and `aria-controls` cannot name an element that is not
|
||||
// in the DOM — which is why the panel renders unconditionally and
|
||||
// `wa-popup` hides it, the same rule `config-section` follows.
|
||||
const el = await fixture<PageHeader>('page-header', {
|
||||
heading: 'Playlists',
|
||||
count: 4,
|
||||
countNoun: 'playlist',
|
||||
sortOptions: SORTS,
|
||||
sortField: 'name',
|
||||
actions: playlistActions([]),
|
||||
});
|
||||
|
||||
await widthOf(el, 480);
|
||||
|
||||
const more = shadow<HTMLButtonElement>(el, '.more-button')!;
|
||||
|
||||
expect(more.hidden).toBe(false);
|
||||
expect(more.getAttribute('aria-label')).toBe('More actions');
|
||||
expect(more.getAttribute('aria-expanded')).toBe('false');
|
||||
expect(more.getAttribute('aria-haspopup')).toBe('menu');
|
||||
|
||||
const panel = shadow<HTMLElement>(el, '#page-header-overflow')!;
|
||||
|
||||
expect(more.getAttribute('aria-controls')).toBe(panel.id);
|
||||
expect(panel.getAttribute('role')).toBe('menu');
|
||||
|
||||
more.click();
|
||||
await el.updateComplete;
|
||||
|
||||
expect(
|
||||
shadow<HTMLButtonElement>(el, '.more-button')!.getAttribute(
|
||||
'aria-expanded',
|
||||
),
|
||||
).toBe('true');
|
||||
});
|
||||
|
||||
it('runs a collapsed action from the menu, and closes it', async () => {
|
||||
const seen: string[] = [];
|
||||
const el = await fixture<PageHeader>('page-header', {
|
||||
heading: 'Playlists',
|
||||
count: 4,
|
||||
countNoun: 'playlist',
|
||||
sortOptions: SORTS,
|
||||
sortField: 'name',
|
||||
actions: playlistActions(seen),
|
||||
});
|
||||
|
||||
await widthOf(el, 700);
|
||||
shadow<HTMLButtonElement>(el, '.more-button')!.click();
|
||||
await el.updateComplete;
|
||||
|
||||
shadowAll<HTMLElement>(el, '#page-header-overflow wa-dropdown-item')[0]!.click();
|
||||
await el.updateComplete;
|
||||
|
||||
expect(seen).toEqual(['import']);
|
||||
expect(
|
||||
shadow<HTMLButtonElement>(el, '.more-button')!.getAttribute(
|
||||
'aria-expanded',
|
||||
),
|
||||
).toBe('false');
|
||||
});
|
||||
|
||||
it('keeps a drop target a drop target, and does not fake one in the menu', async () => {
|
||||
// You cannot drag a track onto a closed menu, so the affordance is
|
||||
// absent from the overflow rather than approximated there. The
|
||||
// header wires the handlers onto the button and owns none of them.
|
||||
const dropped: string[] = [];
|
||||
const actions: PageAction[] = [
|
||||
{
|
||||
id: 'new-playlist',
|
||||
label: 'New Playlist',
|
||||
icon: 'plus',
|
||||
onSelect: () => undefined,
|
||||
drop: {
|
||||
active: true,
|
||||
onDragOver: () => dropped.push('over'),
|
||||
onDragLeave: () => dropped.push('leave'),
|
||||
onDrop: () => dropped.push('drop'),
|
||||
},
|
||||
},
|
||||
];
|
||||
const el = await fixture<PageHeader>('page-header', {
|
||||
heading: 'Playlists',
|
||||
actions,
|
||||
});
|
||||
|
||||
await widthOf(el, 1200);
|
||||
|
||||
const button = shadow<HTMLElement>(
|
||||
el,
|
||||
'[data-testid="page-action-new-playlist"]',
|
||||
)!;
|
||||
|
||||
expect(button.classList.contains('drag-over')).toBe(true);
|
||||
|
||||
button.dispatchEvent(new DragEvent('dragover', { bubbles: true }));
|
||||
button.dispatchEvent(new DragEvent('drop', { bubbles: true }));
|
||||
|
||||
expect(dropped).toEqual(['over', 'drop']);
|
||||
|
||||
// …and collapsed, it is a menu item with no drop wiring at all.
|
||||
await widthOf(el, 120);
|
||||
|
||||
expect(menu(el)).toEqual(['New Playlist']);
|
||||
expect(
|
||||
shadow<HTMLElement>(el, '[data-testid="page-action-new-playlist"]')
|
||||
?.hidden,
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it('renders nothing at all for a view with no actions', async () => {
|
||||
// Two of the three hosts have one action and one has none while its
|
||||
// other tab is up; an empty actions row is not a mode.
|
||||
const el = await fixture<PageHeader>('page-header', { heading: 'Albums' });
|
||||
|
||||
await widthOf(el, 900);
|
||||
|
||||
expect(shadow(el, '.actions')).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('<page-header> as each view wears it', () => {
|
||||
// One baseline per arrangement rather than per view: the point is
|
||||
// that eight views produce four shapes, not eight.
|
||||
|
||||
@@ -0,0 +1,175 @@
|
||||
/**
|
||||
* #24 — the queue stops being a column when it cannot afford to be one.
|
||||
*
|
||||
* In flow the panel is `flex-shrink: 0`, so it takes its width *from
|
||||
* the main panel* rather than covering it. Measured against the running
|
||||
* app on the Playlists page, that left 379px of content at 900×600 —
|
||||
* with all three of the page header's actions clipped — 69px at 390px
|
||||
* wide, and **0px** at 320px, where the content was not degraded but
|
||||
* gone.
|
||||
*
|
||||
* The rule is `available - panelWidth >= MAIN_PANEL_FLOOR`, and the
|
||||
* reason it is a computed property rather than a `@media` block is the
|
||||
* third test here: the panel's width is user state, drag-resizable
|
||||
* between 200 and 500px and persisted, so a breakpoint on the viewport
|
||||
* alone is wrong by up to 180px for a user who has widened it — in the
|
||||
* direction that hurts, since a wider queue is exactly when the content
|
||||
* can least afford it.
|
||||
*
|
||||
* The parent is `.content-area`, i.e. the viewport minus the sidebar,
|
||||
* which is why these mount into a sized wrapper rather than into
|
||||
* `document.body`: the width that decides this is the *parent's*, and
|
||||
* `fixture()` would hand the panel the whole test window.
|
||||
*/
|
||||
import { describe, expect, it, afterEach } from 'vitest';
|
||||
|
||||
import '@components/queue-panel/queue-panel';
|
||||
import type { QueuePanel } from '@components/queue-panel/queue-panel';
|
||||
import { shadow } from '@test/support/render';
|
||||
|
||||
const wrappers: HTMLElement[] = [];
|
||||
|
||||
afterEach(() => {
|
||||
for (const w of wrappers.splice(0)) w.remove();
|
||||
});
|
||||
|
||||
/**
|
||||
* Mount a panel inside a parent of a stated width.
|
||||
*
|
||||
* The wrapper is `position: relative` and `display: flex` because that
|
||||
* is what `.content-area` is; the mode is measured from
|
||||
* `parentElement.clientWidth`, so a wrapper that collapses to its
|
||||
* content would measure the panel rather than the space around it.
|
||||
*/
|
||||
async function panelIn(parentWidth: number): Promise<QueuePanel> {
|
||||
const wrapper = document.createElement('div');
|
||||
|
||||
wrapper.style.cssText = `position: relative; display: flex; width: ${parentWidth}px;`;
|
||||
document.body.append(wrapper);
|
||||
wrappers.push(wrapper);
|
||||
|
||||
const el = document.createElement('queue-panel') as QueuePanel;
|
||||
|
||||
el.open = true;
|
||||
wrapper.append(el);
|
||||
|
||||
await el.updateComplete;
|
||||
await settle(el);
|
||||
|
||||
return el;
|
||||
}
|
||||
|
||||
/**
|
||||
* A ResizeObserver delivers on a frame, not a microtask, so the mode
|
||||
* lands a frame after the width that decides it.
|
||||
*/
|
||||
async function settle(el: QueuePanel): Promise<void> {
|
||||
for (let frame = 0; frame < 4; frame += 1) {
|
||||
await new Promise((r) => {
|
||||
requestAnimationFrame(() => r(null));
|
||||
});
|
||||
await el.updateComplete;
|
||||
}
|
||||
}
|
||||
|
||||
/** Drag the resize handle by `dx`, the way a user widens the panel. */
|
||||
async function dragHandleBy(el: QueuePanel, dx: number): Promise<void> {
|
||||
const handle = shadow<HTMLElement>(el, '.resize-handle');
|
||||
const startX = el.getBoundingClientRect().left;
|
||||
|
||||
if (!handle) throw new Error('no resize handle to drag');
|
||||
|
||||
handle.dispatchEvent(
|
||||
new MouseEvent('mousedown', { clientX: startX, bubbles: true }),
|
||||
);
|
||||
document.dispatchEvent(
|
||||
new MouseEvent('mousemove', { clientX: startX - dx, bubbles: true }),
|
||||
);
|
||||
document.dispatchEvent(new MouseEvent('mouseup', { bubbles: true }));
|
||||
|
||||
await settle(el);
|
||||
}
|
||||
|
||||
describe('the queue panel decides whether it can be a column', () => {
|
||||
it('stays inline while the content can spare the width', async () => {
|
||||
const el = await panelIn(1080);
|
||||
|
||||
expect(el.overlay).toBe(false);
|
||||
expect(el.hasAttribute('overlay')).toBe(false);
|
||||
});
|
||||
|
||||
it('becomes an overlay when it cannot', async () => {
|
||||
const el = await panelIn(700);
|
||||
|
||||
expect(el.overlay).toBe(true);
|
||||
expect(el.hasAttribute('overlay')).toBe(true);
|
||||
});
|
||||
|
||||
/**
|
||||
* The test the media query could not have passed. The parent does not
|
||||
* move; only the user's own panel width does.
|
||||
*/
|
||||
it('flips to overlay when the user widens the panel, at a fixed width', async () => {
|
||||
const el = await panelIn(880);
|
||||
|
||||
expect(el.overlay).toBe(false);
|
||||
|
||||
await dragHandleBy(el, 180);
|
||||
|
||||
expect(el.overlay).toBe(true);
|
||||
});
|
||||
|
||||
it('gives an overlay a scrim and a named way out, and an inline panel neither', async () => {
|
||||
const overlaid = await panelIn(700);
|
||||
|
||||
expect(shadow(overlaid, '.scrim')).toBeTruthy();
|
||||
|
||||
const close = shadow(overlaid, '[data-testid="queue-close"]');
|
||||
|
||||
expect(close?.getAttribute('aria-label')).toBe('Close queue');
|
||||
|
||||
const inline = await panelIn(1080);
|
||||
|
||||
expect(inline.shadowRoot?.querySelector('.scrim')).toBeNull();
|
||||
expect(
|
||||
inline.shadowRoot?.querySelector('[data-testid="queue-close"]'),
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
it('closes on the scrim, on the close button and on Escape', async () => {
|
||||
for (const close of [
|
||||
(el: QueuePanel) => shadow<HTMLElement>(el, '.scrim')?.click(),
|
||||
(el: QueuePanel) =>
|
||||
shadow<HTMLElement>(el, '[data-testid="queue-close"]')?.click(),
|
||||
() =>
|
||||
document.dispatchEvent(
|
||||
new KeyboardEvent('keydown', { key: 'Escape', bubbles: true }),
|
||||
),
|
||||
]) {
|
||||
const el = await panelIn(700);
|
||||
|
||||
expect(el.open).toBe(true);
|
||||
|
||||
close(el);
|
||||
await el.updateComplete;
|
||||
|
||||
expect(el.open).toBe(false);
|
||||
}
|
||||
});
|
||||
|
||||
/**
|
||||
* Escape belongs to the overlay, not to the queue. An inline panel is
|
||||
* beside the content rather than over it, so there is nothing to
|
||||
* dismiss and the key has to reach whatever else wants it.
|
||||
*/
|
||||
it('leaves Escape alone while inline', async () => {
|
||||
const el = await panelIn(1080);
|
||||
|
||||
document.dispatchEvent(
|
||||
new KeyboardEvent('keydown', { key: 'Escape', bubbles: true }),
|
||||
);
|
||||
await el.updateComplete;
|
||||
|
||||
expect(el.open).toBe(true);
|
||||
});
|
||||
});
|
||||
+6
-11
@@ -20,19 +20,14 @@ pre-commit:
|
||||
glob: "*.go"
|
||||
run: go tool golangci-lint run --timeout 5m ./...
|
||||
|
||||
# Snapshots the tree either side of the generators and reports only
|
||||
# what moved across them. This used to be `go generate` plus a bare
|
||||
# `git diff --name-only`, which is the *whole unstaged worktree* — so
|
||||
# any unrelated edit sitting there was reported as stale generated
|
||||
# code, and `make generate` then fixed nothing. See the script.
|
||||
codegen-check:
|
||||
glob: "*.{go,sql,templ}"
|
||||
run: |
|
||||
go generate ./...
|
||||
if [ -n "$(git diff --name-only)" ]; then
|
||||
echo "Generated code is out of date. Run 'make generate' and stage the changes."
|
||||
# --no-pager, or this blocks forever on `less` waiting for a
|
||||
# keypress that a hook run without a tty will never get: the
|
||||
# commit hangs at exactly the moment it is trying to tell you
|
||||
# why it failed.
|
||||
git --no-pager diff --stat
|
||||
exit 1
|
||||
fi
|
||||
run: ./scripts/codegen-check.sh
|
||||
|
||||
# frontend/bindings is generated by `wails3`, not `go generate`, so
|
||||
# the check above does not cover it. ~3.5s warm, ~20s on a cold
|
||||
|
||||
Executable
+81
@@ -0,0 +1,81 @@
|
||||
#!/usr/bin/env bash
|
||||
#
|
||||
# Fails when `go generate ./...` would change something that is not staged.
|
||||
#
|
||||
# The obvious spelling of this is `go generate && git diff --name-only`,
|
||||
# which is what the hook used to be, and it answers the wrong question:
|
||||
# that diff is the *whole unstaged worktree*, so any unrelated edit — a
|
||||
# note, a plan document, the next commit's files sitting there while this
|
||||
# one lands — was reported as
|
||||
#
|
||||
# Generated code is out of date. Run 'make generate' and stage the changes.
|
||||
#
|
||||
# Running `make generate` then does nothing, because nothing generated is
|
||||
# stale, and the message sends you looking for a codegen problem that does
|
||||
# not exist. Splitting one piece of work into several commits is exactly
|
||||
# the shape that triggers it, so the workaround was a constraint on commit
|
||||
# order for no real reason.
|
||||
#
|
||||
# So the tree is snapshotted either side of the generators and only what
|
||||
# *moved across them* is reported. That is deliberately not a list of
|
||||
# generated paths: sqlcgen, `*_templ.go` and `frontend/src/events.ts` are
|
||||
# today's answer, a fourth generator is one `//go:generate` line away, and
|
||||
# a path list is a second place to remember it — the same reasoning that
|
||||
# keeps staleshape.go parsing sql/schemas/ rather than restating it.
|
||||
#
|
||||
# Content, not names: a generated file that is *already* dirty and is then
|
||||
# rewritten further keeps its name in both snapshots and would otherwise
|
||||
# slip through.
|
||||
|
||||
set -euo pipefail
|
||||
|
||||
cd "$(dirname "$0")/.."
|
||||
|
||||
# name + worktree blob hash for every file that differs from the index.
|
||||
# A file listed but absent (a deletion) hashes as "gone" rather than
|
||||
# aborting the pipeline.
|
||||
snapshot() {
|
||||
git diff --name-only | while IFS= read -r f; do
|
||||
if [ -f "$f" ]; then
|
||||
printf '%s %s\n' "$f" "$(git hash-object -- "$f")"
|
||||
else
|
||||
printf '%s gone\n' "$f"
|
||||
fi
|
||||
done
|
||||
}
|
||||
|
||||
# A brand-new generated file is not in either diff, because it is not
|
||||
# tracked at all — the same blind spot bindings-check.sh names. Both
|
||||
# snapshots are taken before the generators run.
|
||||
before="$(snapshot)"
|
||||
before_untracked="$(git ls-files --others --exclude-standard)"
|
||||
|
||||
go generate ./...
|
||||
|
||||
after="$(snapshot)"
|
||||
after_untracked="$(git ls-files --others --exclude-standard)"
|
||||
|
||||
# Symmetric difference, and the symmetry is the whole point. Generation
|
||||
# can push a file *into* the unstaged set (it was current, now it is not)
|
||||
# or *out* of it (someone hand-edited generated output and the generator
|
||||
# put it back) — and the second is stale generated code just as much as
|
||||
# the first. Comparing one direction only reports "current" for it,
|
||||
# which is the failure this script was written to stop.
|
||||
moved="$(comm -3 <(printf '%s\n' "$before" | sort) <(printf '%s\n' "$after" | sort) |
|
||||
cut -d' ' -f1 | tr -d '\t' | sort -u | grep -v '^$' || true)"
|
||||
|
||||
if [ -n "$moved" ]; then
|
||||
echo "codegen-check: generated code is out of date." >&2
|
||||
echo "Run 'make generate' and stage:" >&2
|
||||
printf ' %s\n' $moved >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
if [ "$after_untracked" != "$before_untracked" ]; then
|
||||
echo "codegen-check: generation produced new files. Stage them:" >&2
|
||||
comm -13 <(printf '%s\n' "$before_untracked" | sort) \
|
||||
<(printf '%s\n' "$after_untracked" | sort) >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
echo "codegen-check: generated code is current"
|
||||
@@ -97,6 +97,55 @@ if [ -f "$PID_FILE" ] && kill -0 "$(cat "$PID_FILE")" 2>/dev/null; then
|
||||
fi
|
||||
rm -f "$PID_FILE"
|
||||
|
||||
# ── Refuse to inherit somebody else's port ───────────────────────────
|
||||
# The PID check above only knows about *this* worktree: `make dev-stop`
|
||||
# kills the pid in this .dev/app.pid and nothing else. Several worktrees
|
||||
# of this repo share the default port, so an app orphaned by a deleted
|
||||
# worktree goes on listening with nothing left to stop it.
|
||||
#
|
||||
# Without this check the new app starts, fails to bind, exits — and every
|
||||
# curl and playwright-cli call afterwards goes to the *other* process, so
|
||||
# the harness reports facts about an app nobody asked for. That is not a
|
||||
# quiet wrongness either: it presented as
|
||||
# "no such table: libraries" against a freshly created YJ_HOME, which
|
||||
# reads exactly like applySchema or staleshape.go having gone wrong and
|
||||
# is a frightening place to start looking.
|
||||
#
|
||||
# The startup wait below cannot catch it, because the health check is
|
||||
# satisfied by *any* app on the port — which is precisely the failure.
|
||||
# So it is refused here, before anything is launched, rather than warned
|
||||
# about. --port already exists for the legitimate second-app case.
|
||||
port_holder() {
|
||||
command -v ss >/dev/null || return 0
|
||||
ss -lptn "sport = :$PORT" 2>/dev/null | grep -oP 'pid=\K[0-9]+' | head -n 1
|
||||
}
|
||||
|
||||
if curl -sf -o /dev/null --max-time 2 "http://localhost:$PORT/" ||
|
||||
[ -n "$(port_holder)" ]; then
|
||||
holder="$(port_holder)"
|
||||
echo "dev-headless: :$PORT is already in use; refusing to start" >&2
|
||||
if [ -n "$holder" ]; then
|
||||
# /proc/<pid>/cwd names the checkout it belongs to, and says
|
||||
# "(deleted)" for the orphaned-worktree case that is the whole
|
||||
# reason this is worth a check.
|
||||
cwd="$(readlink "/proc/$holder/cwd" 2>/dev/null || echo unknown)"
|
||||
cmd="$(tr '\0' ' ' <"/proc/$holder/cmdline" 2>/dev/null || echo unknown)"
|
||||
echo " pid $holder ($cmd)" >&2
|
||||
echo " cwd $cwd" >&2
|
||||
# The PID-file check above has already passed, so whatever this
|
||||
# is, `make dev-stop` does not know about it — saying otherwise
|
||||
# sends you to a command that will report success and change
|
||||
# nothing. Never `pkill -f` here either: the pattern would
|
||||
# match this script's own command line.
|
||||
echo " 'make dev-stop' will not touch it (it is not in" >&2
|
||||
echo " ${PID_FILE#"$REPO_ROOT"/}): kill $holder, or pass --port." >&2
|
||||
else
|
||||
echo " The holder could not be identified (no ss, or it belongs" >&2
|
||||
echo " to another user). Try: ss -lptn 'sport = :$PORT'" >&2
|
||||
fi
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# ── Choose the YJ_HOME ───────────────────────────────────────────────
|
||||
# A seed is a YJ_HOME that a previous run of the app produced, tarred
|
||||
# up (see scripts/seed-sandbox.sh). Restoring it means starting *in*
|
||||
@@ -200,6 +249,20 @@ until curl -sf -o /dev/null "http://localhost:$PORT/"; do
|
||||
sleep 0.25
|
||||
done
|
||||
|
||||
# The loop above exits on the first answer from the port, and "something
|
||||
# answered" is not "the app we started answered". The pre-launch guard
|
||||
# makes that unlikely rather than impossible — a race, or a listener
|
||||
# started in between — and the check is one signal, so it is worth making
|
||||
# here too. An empty log beside a dead pid is the "it exited immediately
|
||||
# and nothing said so" case that the original report spent its time on.
|
||||
if ! kill -0 "$APP_PID" 2>/dev/null; then
|
||||
echo "dev-headless: :$PORT answered, but the app we started (pid" >&2
|
||||
echo " $APP_PID) is gone — something else holds the port." >&2
|
||||
tail -n 30 "$LOG_FILE" >&2
|
||||
rm -f "$PID_FILE"
|
||||
exit 1
|
||||
fi
|
||||
|
||||
cat <<EOF
|
||||
dev-headless: up
|
||||
url http://localhost:$PORT
|
||||
|
||||
+27
-3
@@ -38,8 +38,10 @@
|
||||
# Where a body is taken and no --body-file is given, it is read from stdin.
|
||||
#
|
||||
# Environment:
|
||||
# GITEA_TOKEN a PAT with write:issue (plus write:repository and read:user,
|
||||
# which the rest of this repo's tooling reaches for)
|
||||
# GITEA_TOKEN a PAT with write:issue. `claim` and `mine` additionally
|
||||
# need to know your username: set GITEA_USER, or give the
|
||||
# token read:user and it is looked up.
|
||||
# GITEA_USER your Gitea login. Optional; see above.
|
||||
# GITEA_URL defaults to https://git.ljones.me
|
||||
# GITEA_REPO defaults to yonlu/yellowjacket
|
||||
set -euo pipefail
|
||||
@@ -81,7 +83,29 @@ read_body() {
|
||||
if [ "$file" = "-" ]; then cat; else cat "$file"; fi
|
||||
}
|
||||
|
||||
me() { curl -sS -H "Authorization: token $GITEA_TOKEN" "$server/api/v1/user" | python3 "$py" login; }
|
||||
# The one lookup in this script that needs a scope beyond write:issue.
|
||||
# `GET /user` requires read:user, and it is reached for exactly two reasons:
|
||||
# to name the assignee in `claim`, and to filter in `mine`. A token scoped to
|
||||
# the work this script does — write:issue — therefore failed at `claim`, which
|
||||
# is the one step the workflow requires before the first edit, so the whole
|
||||
# documented process was blocked by its own tooling.
|
||||
#
|
||||
# GITEA_USER short-circuits it, which is what lets a least-privilege token do
|
||||
# the job. The lookup stays as the fallback because it is right when the
|
||||
# scope is there and needs no setup at all.
|
||||
me() {
|
||||
if [ -n "${GITEA_USER:-}" ]; then
|
||||
printf '%s' "$GITEA_USER"
|
||||
return
|
||||
fi
|
||||
curl -sS -H "Authorization: token $GITEA_TOKEN" "$server/api/v1/user" |
|
||||
python3 "$py" login ||
|
||||
{
|
||||
echo "issue.sh: could not resolve your username. Set GITEA_USER, or" >&2
|
||||
echo "issue.sh: re-issue GITEA_TOKEN with read:user." >&2
|
||||
exit 1
|
||||
}
|
||||
}
|
||||
|
||||
label_id() { call GET "/labels?limit=100" | python3 "$py" label-id "$1"; }
|
||||
|
||||
|
||||
Reference in New Issue
Block a user