Android: leave the volume to the system, and give Now Playing 29px back #174
@@ -130,6 +130,25 @@ test.describe('the shell on a phone', () => {
|
||||
// are here, and they are the *same* components -- this view
|
||||
// composes the transport rather than reimplementing it.
|
||||
await expect(app.locator('now-playing-view seek-bar')).toBeVisible();
|
||||
|
||||
// Volume is here **because the player says there is one** (#64),
|
||||
// not because this is a phone. This tier is the platform that owns
|
||||
// its own volume, so what it can assert is that the control's
|
||||
// presence follows that answer -- an inverted polarity in
|
||||
// `volume-style-store` fails here and in `bottom-bar.spec.ts`, and
|
||||
// the *absent* branch is checked in the component tier, where the
|
||||
// binding can be stubbed. Nothing here can reach the Android side.
|
||||
const systemOwns = await app.evaluate(
|
||||
async () =>
|
||||
(await window.__yjEvents.call(
|
||||
'player.Player.SystemOwnsVolume',
|
||||
[],
|
||||
5_000,
|
||||
)) as boolean,
|
||||
);
|
||||
|
||||
expect(systemOwns, 'this platform should own its own volume').toBe(false);
|
||||
|
||||
await expect(app.locator('now-playing-view volume-control')).toBeVisible();
|
||||
|
||||
// Back goes where the user came from, through the nav stack.
|
||||
|
||||
+13
-9
@@ -522,16 +522,20 @@ body div.sidebar {
|
||||
}
|
||||
|
||||
/* Volume stands down here whatever the setting says, because this
|
||||
is about room and about the platform rather than about
|
||||
preference: the hardware keys own volume on a phone, which is
|
||||
also why mediacontrols' Android handler implements no volume
|
||||
callback. It moved from `audio-player`'s own media query when
|
||||
#42 moved the control into the bar — same rule, and now stated
|
||||
where the element actually is.
|
||||
is about room: five controls and a slider do not fit a 360px
|
||||
bar, and the full-screen now-playing view is where seeking and
|
||||
volume go on a phone. It moved from `audio-player`'s own media
|
||||
query when #42 moved the control into the bar — same rule, and
|
||||
now stated where the element actually is.
|
||||
|
||||
`.bottom-bar volume-control`, not the one in
|
||||
`now-playing-view`: that view is the phone's transport and is
|
||||
where a slider does belong. */
|
||||
**This rule used to carry the platform argument too, and no
|
||||
longer does** (#64). "The hardware keys own the volume" is not a
|
||||
width: it is false of a narrow desktop window and true of an
|
||||
Android tablet, which this selector gets backwards both ways.
|
||||
The player answers it now — `SystemOwnsVolume` — and
|
||||
`volume-control` renders nothing when it is true, at every
|
||||
width and in both of its mount points. What is left here is the
|
||||
question a stylesheet can actually answer. */
|
||||
.bottom-bar volume-control {
|
||||
display: none;
|
||||
}
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { LitElement, html, css } from 'lit';
|
||||
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/slider/slider.js';
|
||||
@@ -27,6 +27,26 @@ export class VolumeControl extends LitElement {
|
||||
@state()
|
||||
private popup = volumeStyleStore.popup;
|
||||
|
||||
/**
|
||||
* Whether there is a volume of ours to control at all (#64).
|
||||
*
|
||||
* The decision is made here rather than at either mount point,
|
||||
* because there are two -- the bottom bar's copy lives in
|
||||
* `index.html`, which has no module scope to make it conditional --
|
||||
* and one of them is a control the shell cannot un-render. So the
|
||||
* control answers for itself, and the bar and the phone's
|
||||
* full-screen transport get the same answer without either knowing
|
||||
* the question exists.
|
||||
*
|
||||
* It renders `nothing` *and* hides the host: an empty shadow root is
|
||||
* what stops a positional or role query finding a button that cannot
|
||||
* act, and `:host([hidden])` is what stops the element occupying a
|
||||
* flex item's worth of the transport -- the `:host` display above
|
||||
* outranks the UA's `[hidden]` rule, so it has to be said.
|
||||
*/
|
||||
@state()
|
||||
private available = volumeStyleStore.available;
|
||||
|
||||
private unsubscribeStyle?: () => void;
|
||||
|
||||
// Locally-tracked volume while the user is actively dragging or scrolling.
|
||||
@@ -43,6 +63,13 @@ export class VolumeControl extends LitElement {
|
||||
align-items: center;
|
||||
}
|
||||
|
||||
/* See the available field. A gap is only drawn between boxes,
|
||||
so a hidden host costs its parent nothing -- which is where the
|
||||
29px this gives back to Now Playing comes from (#172). */
|
||||
:host([hidden]) {
|
||||
display: none;
|
||||
}
|
||||
|
||||
button {
|
||||
background: none;
|
||||
border: none;
|
||||
@@ -154,12 +181,15 @@ export class VolumeControl extends LitElement {
|
||||
|
||||
this.unsubscribeStyle = volumeStyleStore.subscribe(() => {
|
||||
this.popup = volumeStyleStore.popup;
|
||||
this.setAvailable(volumeStyleStore.available);
|
||||
|
||||
// Switching to the slider while the popup is open would leave the
|
||||
// document listener installed for a popup that no longer renders.
|
||||
if (!this.popup) this.closeSlider();
|
||||
});
|
||||
|
||||
this.setAvailable(volumeStyleStore.available);
|
||||
|
||||
void volumeStyleStore.init();
|
||||
}
|
||||
|
||||
@@ -233,7 +263,25 @@ export class VolumeControl extends LitElement {
|
||||
// RENDER
|
||||
// ===================================================================
|
||||
|
||||
/**
|
||||
* `hidden` is set imperatively rather than reflected from the state,
|
||||
* because it has to be on the *host* and a `@state` does not reflect.
|
||||
* It is the right attribute besides: it takes the element out of the
|
||||
* accessibility tree as well as out of the layout.
|
||||
*/
|
||||
private setAvailable(available: boolean) {
|
||||
this.available = available;
|
||||
this.hidden = !available;
|
||||
|
||||
// A popup left open when the control goes away would keep its
|
||||
// document click listener installed for markup that no longer
|
||||
// renders.
|
||||
if (!available) this.closeSlider();
|
||||
}
|
||||
|
||||
override render() {
|
||||
if (!this.available) return nothing;
|
||||
|
||||
const muted = this.player.muted;
|
||||
|
||||
// Inline, the icon is the mute toggle rather than a disclosure:
|
||||
|
||||
@@ -343,6 +343,12 @@ export class NowPlayingView extends LitElement {
|
||||
a media query because the bottom bar wants a
|
||||
different answer at this same viewport. -->
|
||||
<player-controls context="full"></player-controls>
|
||||
<!-- Rendered unconditionally and absent on its own
|
||||
terms where the device owns the volume (#64): the
|
||||
control asks the player, not this view and not the
|
||||
viewport. A hidden host draws no gap, so that is
|
||||
29px of a 439px screen back to the album art
|
||||
(#172). -->
|
||||
<volume-control></volume-control>
|
||||
</div>
|
||||
`;
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { EventsOn } from '@runtime/runtime';
|
||||
import { GetPopupVolume } from '@go/config/config.js';
|
||||
import { SystemOwnsVolume } from '@go/player/player.js';
|
||||
import { Events } from '../events';
|
||||
|
||||
type Subscriber = () => void;
|
||||
@@ -28,10 +29,37 @@ type Subscriber = () => void;
|
||||
* becomes one. An install that has chosen the popup sees it swap once
|
||||
* on load, which is the cheaper of the two wrong first frames: the
|
||||
* inline slider occupies the space the popup's button would have.
|
||||
*
|
||||
* **`available` is the question one step earlier — whether there is a
|
||||
* volume of ours to draw at all (#64).** On Android the hardware keys
|
||||
* are the volume control and the backend pins its own level at
|
||||
* maximum, so a slider here would move nothing.
|
||||
*
|
||||
* It is asked of the *player* rather than of the viewport, and that is
|
||||
* the whole design decision. Every other stand-down rule in this app
|
||||
* is a width, because a width is what a browser can answer and what
|
||||
* every tier can test — but this one is a property of the build. Keyed
|
||||
* on width instead, an Android tablet at 600px or more would draw the
|
||||
* bottom bar's slider over a pinned level: a control that cannot act,
|
||||
* which `library-status-indicator` settled is worse than none.
|
||||
*
|
||||
* It lives beside `popup` because both answer "what presentation does
|
||||
* the volume control get", both readers are the same two components,
|
||||
* and "none" is a presentation. A second store would be a second
|
||||
* subscription in the same `connectedCallback` saying the same thing.
|
||||
*
|
||||
* The initial value is `true` on the same first-frame rule: there is a
|
||||
* volume on every platform but one, and the platform that pins it sees
|
||||
* the control once at boot and never again in the session — the answer
|
||||
* cannot change while the app runs, so by the time the lazily-mounted
|
||||
* now-playing view exists it has long been settled by the bar's own
|
||||
* copy.
|
||||
*/
|
||||
class VolumeStyleStore {
|
||||
private value = false;
|
||||
|
||||
private hasVolume = true;
|
||||
|
||||
private loaded = false;
|
||||
|
||||
private subscribers = new Set<Subscriber>();
|
||||
@@ -47,13 +75,21 @@ class VolumeStyleStore {
|
||||
return this.value;
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether this app has a volume of its own to control. False where
|
||||
* the device owns it; see the class comment.
|
||||
*/
|
||||
get available(): boolean {
|
||||
return this.hasVolume;
|
||||
}
|
||||
|
||||
/** Reads the setting once. Safe to call from every mount. */
|
||||
async init(): Promise<void> {
|
||||
if (this.loaded) return;
|
||||
|
||||
this.loaded = true;
|
||||
|
||||
await this.refresh();
|
||||
await Promise.all([this.refreshAvailability(), this.refresh()]);
|
||||
}
|
||||
|
||||
subscribe(fn: Subscriber): () => void {
|
||||
@@ -77,6 +113,28 @@ class VolumeStyleStore {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Asked once, not on `GeneralConfigChanged`: this is a property of
|
||||
* the platform the binary was built for and cannot change while
|
||||
* the app is running.
|
||||
*/
|
||||
private async refreshAvailability(): Promise<void> {
|
||||
try {
|
||||
const owned = await SystemOwnsVolume();
|
||||
|
||||
if (owned === !this.hasVolume) return;
|
||||
|
||||
this.hasVolume = !owned;
|
||||
this.notify();
|
||||
} catch (err) {
|
||||
// The control renders, which is the answer on every
|
||||
// platform but one and is the recoverable way to be wrong:
|
||||
// a working control nobody needs, rather than a missing one
|
||||
// somebody does.
|
||||
console.error('failed to ask who owns the volume', err);
|
||||
}
|
||||
}
|
||||
|
||||
private notify(): void {
|
||||
for (const fn of this.subscribers) fn();
|
||||
}
|
||||
|
||||
@@ -0,0 +1,93 @@
|
||||
/**
|
||||
* Who owns the volume, and what the control does when it is not us
|
||||
* (#64).
|
||||
*
|
||||
* **This is the tier that can exercise the Android branch**, and it is
|
||||
* the reason the predicate is a backend answer rather than a build tag
|
||||
* the frontend cannot see: `SystemOwnsVolume` is a stub here, so the
|
||||
* "no volume" rendering is checked on an ordinary Linux CI runner with
|
||||
* no device anywhere. What no tier here can check is the *constant*
|
||||
* behind it, which `TestPlatformVolumeOwnershipIsDeclaredOncePerPlatform`
|
||||
* sweeps the Go source for instead.
|
||||
*
|
||||
* It is a file of its own because `volumeStyleStore` asks once and
|
||||
* latches — the answer is a property of the binary and cannot change
|
||||
* while the app runs, so there is deliberately no event that refreshes
|
||||
* it. Vitest gives each file its own module registry, which is what
|
||||
* lets the stub be in place before the singleton is first touched.
|
||||
* The *available* case is the rest of `transport.test.ts`, which mounts
|
||||
* the same element under the default stub.
|
||||
*/
|
||||
import { describe, expect, it, beforeEach } from 'vitest';
|
||||
|
||||
import '@components/audio-player/volume-control/volume-control';
|
||||
import '@components/now-playing-view/now-playing-view';
|
||||
import { Events } from '../../src/events';
|
||||
import { emit, resetHarness, stub } from '@test/support/harness';
|
||||
import { fixture, shadow, shadowAll } from '@test/support/render';
|
||||
import type { TrackInfo } from '@store/player-store';
|
||||
|
||||
const TRACK: TrackInfo = {
|
||||
fileName: 'tideline.mp3',
|
||||
filePath: '/music/tideline.mp3',
|
||||
trackLength: 245,
|
||||
seekPosition: 0,
|
||||
state: 'playing',
|
||||
title: 'Tideline',
|
||||
artist: 'Sea Change',
|
||||
album: 'Ebb',
|
||||
coverArt: '',
|
||||
coverArtSmall: '',
|
||||
coverArtMedium: '',
|
||||
coverArtLarge: '',
|
||||
trackChangeId: 1,
|
||||
artistMbid: '',
|
||||
releaseGroupMbid: '',
|
||||
recordingMbid: '',
|
||||
};
|
||||
|
||||
describe('a platform whose volume we do not own', () => {
|
||||
beforeEach(async () => {
|
||||
resetHarness();
|
||||
stub('player.Player.SystemOwnsVolume', true);
|
||||
stub('config.Config.GetPopupVolume', false);
|
||||
|
||||
// The store latches on the first mount; do it here so every test
|
||||
// below sees a settled answer rather than the first frame.
|
||||
const warm = await fixture('volume-control');
|
||||
|
||||
await warm.updateComplete;
|
||||
});
|
||||
|
||||
it('renders no control at all, and no empty shadow root to find', async () => {
|
||||
const el = await fixture('volume-control');
|
||||
|
||||
await el.updateComplete;
|
||||
|
||||
// Both halves matter. An empty shadow root is what stops a
|
||||
// positional or by-role query finding a button that cannot act;
|
||||
// `hidden` is what stops the host taking a flex item's worth of
|
||||
// space in the transport it sits in.
|
||||
expect(shadowAll(el, 'button')).toHaveLength(0);
|
||||
expect(shadowAll(el, 'wa-slider')).toHaveLength(0);
|
||||
expect(el.hidden, 'the host is not hidden').toBe(true);
|
||||
});
|
||||
|
||||
it('leaves the rest of the phone transport alone', async () => {
|
||||
emit(Events.TrackChanged, TRACK);
|
||||
|
||||
const view = await fixture('now-playing-view');
|
||||
|
||||
await view.updateComplete;
|
||||
|
||||
// Seeking and the transport buttons are not volume, and #64 is
|
||||
// allowed to remove one control, not to thin the screen out.
|
||||
expect(shadow(view, 'seek-bar')).not.toBeNull();
|
||||
expect(shadow(view, 'player-controls')).not.toBeNull();
|
||||
|
||||
const volume = shadow(view, 'volume-control') as HTMLElement | null;
|
||||
|
||||
expect(volume, 'the element is still mounted').not.toBeNull();
|
||||
expect(volume!.hidden, 'a mounted volume-control is not hidden').toBe(true);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user