mirror of
https://github.com/Psychotoxical/psysonic.git
synced 2026-07-22 07:15:47 +00:00
UI/CSS fixes: focus rings, search fields, column dropdown, theme accordion (#954)
* fix(focus): keyboard focus ring no longer clipped by overflow or cover The global :focus-visible ring used a positive outline-offset, so it was drawn outside the element and clipped by any ancestor with overflow:hidden or a scroll container (cards, rails, player bar, queue strip). Draw it inset instead via shared --focus-ring-* variables (single source). Cards draw the ring as an overlay above the cover, since a cover's own stacking context (transform/contain for render stability) would otherwise paint over an inset outline. Dracula now only sets --focus-ring-color. * refactor(focus): fold scattered focus-ring overrides onto shared knob Six components declared their own :focus-visible outline (mostly an exact copy of the old global ring). Remove the redundant ones so they inherit the global inset ring; keep the custom-coloured ones (genre pill, playback-delay modal) but source width/offset from --focus-ring-*; move the because-card ring to the central card focus ring (it has a cover and needs the lifted treatment). * fix(settings): round theme accordion inner box to match its section The theme picker's inner accordion had square corners, so the first (open) group header ran flush into the rounded Theme card. Give .theme-accordion a border-radius + overflow:hidden so its top/bottom corners continue the parent card's rounding, matching the other settings sections. * fix(search): unify search fields to one look Live-search, Help and Settings search differed (pill vs rounded-rect, glow vs plain border-change, mismatched backgrounds). Align them on the canonical input look: radius-md, ctp-base background, accent border + soft accent-dim focus glow. Drop the live-search pill radius, and suppress the input's own inset ring so only the outer cluster glow shows (no double ring). * fix(tracklist): column picker menu no longer clipped on short lists The column-visibility dropdown was an absolutely-positioned menu inside the tracklist, so a short list (e.g. a one-song Favorites view) clipped it via the ancestor's overflow box. Render the menu in a portal to <body> with fixed positioning anchored to the trigger (flips above when there's no room below), following on scroll/resize. Outside-click + Escape close now live in the shared TracklistColumnPicker (the menu is portalled out of the wrapper, so the old wrapper-only check would have closed it on every in-menu click). Fixes albums, playlists and favorites in one shared place. Adds a behaviour test. * docs(changelog): UI/CSS fixes pass (#954)
This commit is contained in:
Frank Stellmacher
committed by
GitHub
GitHub
parent
cc04a0c93d
commit
40932d28e2
@@ -0,0 +1,103 @@
|
||||
import { describe, it, expect, vi } from 'vitest';
|
||||
import React, { useRef, useState } from 'react';
|
||||
import { screen, fireEvent } from '@testing-library/react';
|
||||
import userEvent from '@testing-library/user-event';
|
||||
import type { TFunction } from 'i18next';
|
||||
import { renderWithProviders } from '@/test/helpers/renderWithProviders';
|
||||
import { TracklistColumnPicker } from './TracklistColumnPicker';
|
||||
import type { ColDef } from '@/utils/useTracklistColumns';
|
||||
|
||||
const COLUMNS: readonly ColDef[] = [
|
||||
{ key: 'title', i18nKey: 'title', minWidth: 100, defaultWidth: 200, required: true },
|
||||
{ key: 'artist', i18nKey: 'artist', minWidth: 80, defaultWidth: 150, required: false },
|
||||
{ key: 'album', i18nKey: 'album', minWidth: 80, defaultWidth: 150, required: false },
|
||||
];
|
||||
|
||||
// Passthrough translator: returns the key so assertions are translation-agnostic.
|
||||
const passthroughT = ((k: string) => k) as unknown as TFunction;
|
||||
|
||||
function Harness({
|
||||
initialOpen = false,
|
||||
toggleColumn = vi.fn(),
|
||||
resetColumns = vi.fn(),
|
||||
}: {
|
||||
initialOpen?: boolean;
|
||||
toggleColumn?: (key: string) => void;
|
||||
resetColumns?: () => void;
|
||||
}) {
|
||||
const [open, setOpen] = useState(initialOpen);
|
||||
const pickerRef = useRef<HTMLDivElement>(null);
|
||||
return (
|
||||
<TracklistColumnPicker
|
||||
allColumns={COLUMNS}
|
||||
pickerRef={pickerRef}
|
||||
pickerOpen={open}
|
||||
setPickerOpen={setOpen}
|
||||
colVisible={new Set(['title', 'artist', 'album'])}
|
||||
toggleColumn={toggleColumn}
|
||||
resetColumns={resetColumns}
|
||||
t={passthroughT}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
||||
describe('TracklistColumnPicker', () => {
|
||||
it('opens the menu from the trigger and lists only non-required columns', async () => {
|
||||
renderWithProviders(<Harness />);
|
||||
|
||||
// Closed: only the trigger button exists.
|
||||
await userEvent.click(screen.getByRole('button'));
|
||||
|
||||
expect(screen.getByText('albumDetail.columns')).toBeInTheDocument();
|
||||
expect(screen.getByText('albumDetail.artist')).toBeInTheDocument();
|
||||
expect(screen.getByText('albumDetail.album')).toBeInTheDocument();
|
||||
// Required columns are not offered in the menu.
|
||||
expect(screen.queryByText('albumDetail.title')).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('toggles a column without closing the menu', async () => {
|
||||
const toggleColumn = vi.fn();
|
||||
renderWithProviders(<Harness initialOpen toggleColumn={toggleColumn} />);
|
||||
|
||||
await userEvent.click(screen.getByText('albumDetail.artist'));
|
||||
|
||||
expect(toggleColumn).toHaveBeenCalledWith('artist');
|
||||
// Menu stays open so several columns can be toggled in one pass.
|
||||
expect(screen.getByText('albumDetail.columns')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('calls resetColumns from the reset action', async () => {
|
||||
const resetColumns = vi.fn();
|
||||
renderWithProviders(<Harness initialOpen resetColumns={resetColumns} />);
|
||||
|
||||
await userEvent.click(screen.getByText('albumDetail.resetColumns'));
|
||||
|
||||
expect(resetColumns).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
it('closes on Escape', () => {
|
||||
renderWithProviders(<Harness initialOpen />);
|
||||
expect(screen.getByText('albumDetail.columns')).toBeInTheDocument();
|
||||
|
||||
fireEvent.keyDown(document, { key: 'Escape' });
|
||||
|
||||
expect(screen.queryByText('albumDetail.columns')).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('closes on an outside mousedown', () => {
|
||||
renderWithProviders(<Harness initialOpen />);
|
||||
expect(screen.getByText('albumDetail.columns')).toBeInTheDocument();
|
||||
|
||||
fireEvent.mouseDown(document.body);
|
||||
|
||||
expect(screen.queryByText('albumDetail.columns')).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('stays open on a mousedown inside the menu', () => {
|
||||
renderWithProviders(<Harness initialOpen />);
|
||||
|
||||
fireEvent.mouseDown(screen.getByText('albumDetail.artist'));
|
||||
|
||||
expect(screen.getByText('albumDetail.columns')).toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
@@ -1,4 +1,5 @@
|
||||
import React from 'react';
|
||||
import React, { useEffect, useLayoutEffect, useRef, useState } from 'react';
|
||||
import { createPortal } from 'react-dom';
|
||||
import { Check, ChevronDown, RotateCcw } from 'lucide-react';
|
||||
import type { TFunction } from 'i18next';
|
||||
import type { ColDef } from '../../utils/useTracklistColumns';
|
||||
@@ -16,10 +17,12 @@ interface Props {
|
||||
}
|
||||
|
||||
/**
|
||||
* The column visibility dropdown that sits outside `.tracklist` so the
|
||||
* popover menu can grow without being clipped by the tracklist's overflow
|
||||
* box. Lists every non-required column and offers a reset-to-defaults
|
||||
* button.
|
||||
* The column-visibility dropdown for tracklists. The menu is rendered in a
|
||||
* portal with fixed positioning anchored to the trigger button, so it can never
|
||||
* be clipped by an ancestor's overflow box — the reason short lists (e.g. a
|
||||
* one-song Favorites list) used to cut it off. Shared by every tracklist
|
||||
* (albums, playlists, favorites); all popover behaviour lives here, so the
|
||||
* pages only wire up column state.
|
||||
*/
|
||||
export function TracklistColumnPicker({
|
||||
allColumns,
|
||||
@@ -31,18 +34,76 @@ export function TracklistColumnPicker({
|
||||
resetColumns,
|
||||
t,
|
||||
}: Props) {
|
||||
const btnRef = useRef<HTMLButtonElement>(null);
|
||||
const popRef = useRef<HTMLDivElement>(null);
|
||||
// Start off-screen until measured so the portal never flashes at 0,0.
|
||||
const [menuStyle, setMenuStyle] = useState<React.CSSProperties>({
|
||||
position: 'fixed', top: -9999, left: -9999,
|
||||
});
|
||||
|
||||
const updatePos = () => {
|
||||
const btn = btnRef.current;
|
||||
if (!btn) return;
|
||||
const MARGIN = 8;
|
||||
const r = btn.getBoundingClientRect();
|
||||
const menuH = popRef.current?.offsetHeight ?? 240;
|
||||
const spaceBelow = window.innerHeight - r.bottom;
|
||||
// Flip above the trigger when there isn't room below and there is above.
|
||||
const openUp = spaceBelow < menuH + MARGIN && r.top > spaceBelow;
|
||||
setMenuStyle({
|
||||
position: 'fixed',
|
||||
right: Math.max(MARGIN, window.innerWidth - r.right),
|
||||
zIndex: 10050,
|
||||
...(openUp
|
||||
? { bottom: window.innerHeight - r.top + 4 }
|
||||
: { top: r.bottom + 4 }),
|
||||
});
|
||||
};
|
||||
|
||||
// Position before paint so the menu appears in place.
|
||||
useLayoutEffect(() => {
|
||||
if (pickerOpen) updatePos();
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
}, [pickerOpen]);
|
||||
|
||||
// Keep anchored on scroll/resize, and close on outside-click / Escape.
|
||||
useEffect(() => {
|
||||
if (!pickerOpen) return;
|
||||
const reposition = () => updatePos();
|
||||
const onDown = (e: MouseEvent) => {
|
||||
const target = e.target as Node;
|
||||
if (btnRef.current?.contains(target) || popRef.current?.contains(target)) return;
|
||||
setPickerOpen(() => false);
|
||||
};
|
||||
const onKey = (e: KeyboardEvent) => {
|
||||
if (e.key === 'Escape') setPickerOpen(() => false);
|
||||
};
|
||||
window.addEventListener('resize', reposition);
|
||||
window.addEventListener('scroll', reposition, true);
|
||||
document.addEventListener('mousedown', onDown);
|
||||
document.addEventListener('keydown', onKey);
|
||||
return () => {
|
||||
window.removeEventListener('resize', reposition);
|
||||
window.removeEventListener('scroll', reposition, true);
|
||||
document.removeEventListener('mousedown', onDown);
|
||||
document.removeEventListener('keydown', onKey);
|
||||
};
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
}, [pickerOpen]);
|
||||
|
||||
return (
|
||||
<div className="tracklist-col-picker-wrapper" ref={pickerRef}>
|
||||
<div className="tracklist-col-picker">
|
||||
<button
|
||||
ref={btnRef}
|
||||
className="tracklist-col-picker-btn"
|
||||
onClick={e => { e.stopPropagation(); setPickerOpen(v => !v); }}
|
||||
data-tooltip={t('albumDetail.columns')}
|
||||
>
|
||||
<ChevronDown size={14} />
|
||||
</button>
|
||||
{pickerOpen && (
|
||||
<div className="tracklist-col-picker-menu">
|
||||
{pickerOpen && createPortal(
|
||||
<div className="tracklist-col-picker-menu" ref={popRef} style={menuStyle}>
|
||||
<div className="tracklist-col-picker-label">{t('albumDetail.columns')}</div>
|
||||
{allColumns.filter(c => !c.required).map(c => {
|
||||
const label = c.i18nKey ? t(`albumDetail.${c.i18nKey as string}`) : c.key;
|
||||
@@ -65,7 +126,8 @@ export function TracklistColumnPicker({
|
||||
<RotateCcw size={13} />
|
||||
{t('albumDetail.resetColumns')}
|
||||
</button>
|
||||
</div>
|
||||
</div>,
|
||||
document.body,
|
||||
)}
|
||||
</div>
|
||||
</div>
|
||||
|
||||
Reference in New Issue
Block a user