forked from akai/readest
* fix(rsvp): restore in-flow control bar layout reverted by #4589 PR #4585 fixed the mobile RSVP control bar overlap by laying the audio toggle and settings gear in a single in-flow flex row flanking the centered transport. PR #4589 branched from main about five minutes before #4585 merged and merged about ten hours later without rebasing, so its squash carried the stale pre-#4585 file and reverted the entire fix, including the regression test #4585 had added. On narrow phones (360px) the audio and settings icons again overlapped the right end of the transport, hiding the "skip forward 15" control. Restore the #4585 layout and re-add a structural guard test asserting the audio toggle and settings share the transport row and live in no absolutely positioned cluster. Verified on a Xiaomi 13 (360px) via on-device CDP: no overlap, play button stays centered. Also stage the project-memory note for this regression. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(rsvp): hide Faster/Slower buttons at 350px or below On very narrow phones (width 350px or less) the control row has no room for every control. Collapse the Faster/Slower speed buttons via a max-[350px]:hidden variant (matching the existing 350px tightening tier) so the transport, audio toggle and settings never overflow. Speed stays adjustable from the WPM dropdown. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Huang Xin
@@ -529,6 +529,75 @@ describe('RSVPOverlay — dictionary lookup (#4475)', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('RSVPOverlay — playback control layout (#4585, regressed by #4589)', () => {
|
||||
afterEach(() => cleanup());
|
||||
|
||||
// The audio (TTS) toggle and the settings gear must sit in the SAME flex row
|
||||
// as the transport buttons, flanking the centered play button in normal flow.
|
||||
// The earlier `absolute end-0` cluster overlaid them on top of the right end
|
||||
// of the transport, hiding the audio button behind "skip forward 15" on narrow
|
||||
// phones. Keeping all three as siblings of the play button is what prevents the
|
||||
// overlap, so assert the structure here (jsdom can't measure the overlap).
|
||||
test('audio toggle and settings flank the transport in the same flex row', () => {
|
||||
const state = buildState({
|
||||
words: [{ text: 'hello', orpIndex: 1, pauseMultiplier: 1 }],
|
||||
currentIndex: 0,
|
||||
});
|
||||
const { container } = renderOverlay(state);
|
||||
|
||||
const audioButton = container.querySelector('[aria-label="Play audio"]') as HTMLElement;
|
||||
const settingsButton = container.querySelector('[aria-label="Settings"]') as HTMLElement;
|
||||
const playButton = container.querySelector('[aria-label="Play"]') as HTMLElement;
|
||||
expect(audioButton).not.toBeNull();
|
||||
expect(settingsButton).not.toBeNull();
|
||||
expect(playButton).not.toBeNull();
|
||||
|
||||
// All three share the play button's parent (the single flex row) — the audio
|
||||
// toggle and settings are not tucked into a separate absolute cluster.
|
||||
expect(audioButton.parentElement).toBe(playButton.parentElement);
|
||||
expect(settingsButton.parentElement).toBe(playButton.parentElement);
|
||||
});
|
||||
|
||||
// On very narrow phones (< 350px) the row has no room for every control, so
|
||||
// the Faster/Slower speed buttons collapse to save space (speed is still
|
||||
// adjustable from the WPM dropdown). The core transport stays put.
|
||||
test('hides the Faster/Slower buttons below 350px to save space', () => {
|
||||
const state = buildState({
|
||||
words: [{ text: 'hello', orpIndex: 1, pauseMultiplier: 1 }],
|
||||
currentIndex: 0,
|
||||
});
|
||||
const { container } = renderOverlay(state);
|
||||
|
||||
const decrease = container.querySelector('[aria-label="Decrease speed"]') as HTMLElement;
|
||||
const increase = container.querySelector('[aria-label="Increase speed"]') as HTMLElement;
|
||||
const play = container.querySelector('[aria-label="Play"]') as HTMLElement;
|
||||
const audio = container.querySelector('[aria-label="Play audio"]') as HTMLElement;
|
||||
const settings = container.querySelector('[aria-label="Settings"]') as HTMLElement;
|
||||
|
||||
expect(decrease.className).toContain('max-[350px]:hidden');
|
||||
expect(increase.className).toContain('max-[350px]:hidden');
|
||||
// Transport, audio toggle and settings must remain visible at any width.
|
||||
expect(play.className).not.toContain('max-[350px]:hidden');
|
||||
expect(audio.className).not.toContain('max-[350px]:hidden');
|
||||
expect(settings.className).not.toContain('max-[350px]:hidden');
|
||||
});
|
||||
|
||||
// The previous fix relied on absolute positioning being gone; guard against it
|
||||
// sneaking back into the row that holds the transport controls.
|
||||
test('the playback control row is not absolutely positioned', () => {
|
||||
const state = buildState({
|
||||
words: [{ text: 'hello', orpIndex: 1, pauseMultiplier: 1 }],
|
||||
currentIndex: 0,
|
||||
});
|
||||
const { container } = renderOverlay(state);
|
||||
|
||||
const playButton = container.querySelector('[aria-label="Play"]') as HTMLElement;
|
||||
const audioButton = container.querySelector('[aria-label="Play audio"]') as HTMLElement;
|
||||
expect(playButton.parentElement!.className).not.toContain('absolute');
|
||||
expect(audioButton.parentElement!.className).not.toContain('absolute');
|
||||
});
|
||||
});
|
||||
|
||||
describe('RSVPOverlay — start delay setting (#4478)', () => {
|
||||
afterEach(() => {
|
||||
cleanup();
|
||||
|
||||
@@ -1077,11 +1077,41 @@ const RSVPOverlay: React.FC<RSVPOverlayProps> = ({
|
||||
</div>
|
||||
</div>
|
||||
|
||||
{/* Playback controls */}
|
||||
<div className='relative flex items-center justify-center gap-1 md:gap-2'>
|
||||
{/* Playback controls — a single full-width flex row on mobile so the
|
||||
audio toggle (far left) and settings gear (far right) flank the
|
||||
centered transport in normal flow instead of overlapping it from an
|
||||
absolute corner; a centered cluster on md+ where there is room. */}
|
||||
<div className='flex items-center justify-between md:justify-center md:gap-2'>
|
||||
{/* Audio (TTS) read-along toggle — starts TTS from the displayed word,
|
||||
or stops it when engaged (decision 5, #3235). Far-left peer of the
|
||||
transport so the centered play button stays centered and nothing
|
||||
overlaps on mobile. Active state uses a filled glyph + eink-bordered
|
||||
surface so it reads in e-ink without relying on color. */}
|
||||
<button
|
||||
aria-label={ttsActive ? _('Pause audio') : _('Play audio')}
|
||||
className={clsx(
|
||||
'touch-target flex h-8 w-8 shrink-0 cursor-pointer items-center justify-center rounded-full border-none transition-colors active:scale-95 md:h-9 md:w-9',
|
||||
ttsActive
|
||||
? 'eink-bordered bg-[color-mix(in_srgb,var(--rsvp-accent)_18%,transparent)]'
|
||||
: 'bg-transparent hover:bg-gray-500/20',
|
||||
)}
|
||||
onClick={() => onToggleTtsAudio?.()}
|
||||
title={ttsActive ? _('Pause audio') : _('Play audio')}
|
||||
>
|
||||
{ttsActive ? (
|
||||
<IoVolumeHigh
|
||||
className='h-4 w-4 md:h-5 md:w-5'
|
||||
style={{ color: accentColor }}
|
||||
aria-hidden='true'
|
||||
/>
|
||||
) : (
|
||||
<IoVolumeMediumOutline className='h-4 w-4 md:h-5 md:w-5' aria-hidden='true' />
|
||||
)}
|
||||
</button>
|
||||
|
||||
<button
|
||||
aria-label={_('Skip back 15 words')}
|
||||
className='flex cursor-pointer items-center gap-0.5 rounded-full border-none bg-transparent px-2 py-1.5 transition-colors hover:bg-gray-500/20 active:scale-95'
|
||||
className='flex shrink-0 cursor-pointer items-center gap-0.5 rounded-full border-none bg-transparent px-1.5 py-1.5 transition-colors hover:bg-gray-500/20 active:scale-95 md:px-2'
|
||||
onClick={() => controller.skipBackward(15)}
|
||||
title={_('Back 15 words (Shift+Left)')}
|
||||
>
|
||||
@@ -1089,9 +1119,11 @@ const RSVPOverlay: React.FC<RSVPOverlayProps> = ({
|
||||
<IoPlaySkipBack className='h-5 w-5 md:h-6 md:w-6' />
|
||||
</button>
|
||||
|
||||
{/* Faster/Slower collapse below 350px (speed is still adjustable from
|
||||
the WPM dropdown) so the transport never overflows the row. */}
|
||||
<button
|
||||
aria-label={_('Decrease speed')}
|
||||
className='flex h-9 w-9 cursor-pointer items-center justify-center rounded-full border-none bg-transparent transition-colors hover:bg-gray-500/20 active:scale-95'
|
||||
className='flex h-8 w-8 shrink-0 cursor-pointer items-center justify-center rounded-full border-none bg-transparent transition-colors hover:bg-gray-500/20 active:scale-95 max-[350px]:hidden md:h-9 md:w-9'
|
||||
onClick={() => controller.decreaseSpeed()}
|
||||
title={_('Slower (Left/Down)')}
|
||||
>
|
||||
@@ -1100,7 +1132,7 @@ const RSVPOverlay: React.FC<RSVPOverlayProps> = ({
|
||||
|
||||
<button
|
||||
aria-label={_('Previous word')}
|
||||
className='flex h-9 w-9 cursor-pointer items-center justify-center rounded-full border-none bg-transparent transition-colors hover:bg-gray-500/20 active:scale-95'
|
||||
className='flex h-8 w-8 shrink-0 cursor-pointer items-center justify-center rounded-full border-none bg-transparent transition-colors hover:bg-gray-500/20 active:scale-95 md:h-9 md:w-9'
|
||||
onClick={() => controller.prevWord()}
|
||||
title={_('Previous word (,)')}
|
||||
>
|
||||
@@ -1110,7 +1142,7 @@ const RSVPOverlay: React.FC<RSVPOverlayProps> = ({
|
||||
<button
|
||||
aria-label={transportPlaying ? _('Pause') : _('Play')}
|
||||
className={clsx(
|
||||
'flex h-14 w-14 cursor-pointer items-center justify-center rounded-full border-none bg-gray-500/15 transition-colors hover:bg-gray-500/25 active:scale-95 md:h-16 md:w-16',
|
||||
'flex h-14 w-14 shrink-0 cursor-pointer items-center justify-center rounded-full border-none bg-gray-500/15 transition-colors hover:bg-gray-500/25 active:scale-95 md:h-16 md:w-16',
|
||||
transportPlaying ? '' : 'ps-1',
|
||||
)}
|
||||
onClick={() => transportToggleRef.current()}
|
||||
@@ -1125,7 +1157,7 @@ const RSVPOverlay: React.FC<RSVPOverlayProps> = ({
|
||||
|
||||
<button
|
||||
aria-label={_('Next word')}
|
||||
className='flex h-9 w-9 cursor-pointer items-center justify-center rounded-full border-none bg-transparent transition-colors hover:bg-gray-500/20 active:scale-95'
|
||||
className='flex h-8 w-8 shrink-0 cursor-pointer items-center justify-center rounded-full border-none bg-transparent transition-colors hover:bg-gray-500/20 active:scale-95 md:h-9 md:w-9'
|
||||
onClick={() => controller.nextWord()}
|
||||
title={_('Next word (.)')}
|
||||
>
|
||||
@@ -1134,7 +1166,7 @@ const RSVPOverlay: React.FC<RSVPOverlayProps> = ({
|
||||
|
||||
<button
|
||||
aria-label={_('Increase speed')}
|
||||
className='flex h-9 w-9 cursor-pointer items-center justify-center rounded-full border-none bg-transparent transition-colors hover:bg-gray-500/20 active:scale-95'
|
||||
className='flex h-8 w-8 shrink-0 cursor-pointer items-center justify-center rounded-full border-none bg-transparent transition-colors hover:bg-gray-500/20 active:scale-95 max-[350px]:hidden md:h-9 md:w-9'
|
||||
onClick={() => controller.increaseSpeed()}
|
||||
title={_('Faster (Right/Up)')}
|
||||
>
|
||||
@@ -1143,7 +1175,7 @@ const RSVPOverlay: React.FC<RSVPOverlayProps> = ({
|
||||
|
||||
<button
|
||||
aria-label={_('Skip forward 15 words')}
|
||||
className='flex cursor-pointer items-center gap-0.5 rounded-full border-none bg-transparent px-2 py-1.5 transition-colors hover:bg-gray-500/20 active:scale-95'
|
||||
className='flex shrink-0 cursor-pointer items-center gap-0.5 rounded-full border-none bg-transparent px-1.5 py-1.5 transition-colors hover:bg-gray-500/20 active:scale-95 md:px-2'
|
||||
onClick={() => controller.skipForward(15)}
|
||||
title={_('Forward 15 words (Shift+Right)')}
|
||||
>
|
||||
@@ -1151,48 +1183,20 @@ const RSVPOverlay: React.FC<RSVPOverlayProps> = ({
|
||||
<span className='text-xs font-semibold opacity-80'>15</span>
|
||||
</button>
|
||||
|
||||
{/* Trailing cluster: audio (TTS) toggle + divider + settings gear.
|
||||
The audio toggle starts TTS from the displayed word (or stops it
|
||||
when engaged) — never a second play triangle (decision 5). Active
|
||||
state uses a filled glyph + eink-bordered surface so it reads in
|
||||
e-ink without relying on color. */}
|
||||
<div className='absolute end-0 flex items-center gap-1'>
|
||||
<button
|
||||
aria-label={ttsActive ? _('Pause audio') : _('Play audio')}
|
||||
className={clsx(
|
||||
'touch-target flex h-9 w-9 cursor-pointer items-center justify-center rounded-full border-none transition-colors active:scale-95',
|
||||
ttsActive
|
||||
? 'eink-bordered bg-[color-mix(in_srgb,var(--rsvp-accent)_18%,transparent)]'
|
||||
: 'bg-transparent hover:bg-gray-500/20',
|
||||
)}
|
||||
onClick={() => onToggleTtsAudio?.()}
|
||||
title={ttsActive ? _('Pause audio') : _('Play audio')}
|
||||
>
|
||||
{ttsActive ? (
|
||||
<IoVolumeHigh
|
||||
className='h-4 w-4 md:h-5 md:w-5'
|
||||
style={{ color: accentColor }}
|
||||
aria-hidden='true'
|
||||
/>
|
||||
) : (
|
||||
<IoVolumeMediumOutline className='h-4 w-4 md:h-5 md:w-5' aria-hidden='true' />
|
||||
)}
|
||||
</button>
|
||||
|
||||
<span className='h-5 w-px bg-gray-500/30' aria-hidden='true' />
|
||||
|
||||
<button
|
||||
aria-label={_('Settings')}
|
||||
className={clsx(
|
||||
'flex h-9 w-9 cursor-pointer items-center justify-center rounded-full border-none bg-transparent transition-colors hover:bg-gray-500/20 active:scale-95',
|
||||
showSettings && 'bg-gray-500/15',
|
||||
)}
|
||||
onClick={() => setShowSettings((prev) => !prev)}
|
||||
title={_('Settings')}
|
||||
>
|
||||
<IoSettingsSharp className='h-4 w-4 md:h-5 md:w-5' />
|
||||
</button>
|
||||
</div>
|
||||
{/* Settings — far-right peer mirroring the audio toggle on the left,
|
||||
so both flank the centered transport in normal flow (decision 5,
|
||||
#3235) without an absolute cluster overlapping it on mobile. */}
|
||||
<button
|
||||
aria-label={_('Settings')}
|
||||
className={clsx(
|
||||
'flex h-8 w-8 shrink-0 cursor-pointer items-center justify-center rounded-full border-none bg-transparent transition-colors hover:bg-gray-500/20 active:scale-95 md:h-9 md:w-9',
|
||||
showSettings && 'bg-gray-500/15',
|
||||
)}
|
||||
onClick={() => setShowSettings((prev) => !prev)}
|
||||
title={_('Settings')}
|
||||
>
|
||||
<IoSettingsSharp className='h-4 w-4 md:h-5 md:w-5' />
|
||||
</button>
|
||||
</div>
|
||||
|
||||
{/* Settings row (collapsible) */}
|
||||
|
||||
Reference in New Issue
Block a user