mirror of
https://github.com/cpvalente/ontime.git
synced 2026-08-27 01:49:10 +00:00
Reuse existing components and drop duplicated definitions
Follow up on the drawer polish, replacing things it hand-rolled with what the codebase already provides. Reuse - The navigation toggles now use the Switch component instead of ~30 lines of CSS mirroring it, which carried a comment admitting the metrics were kept in sync by hand. base-ui renders Switch.Root as a span, so the row can be a label wrapping a real control, the same shape the params editor already uses. The row stays clickable across its full width - Remove scrollbar-width and scrollbar-color from three drawers. index.scss already sets both on the universal selector, so these were not only redundant, they overrode the app wide thumb and track tokens with local values - Collect the user facing view names in one map in viewerConfig, and derive the navigation entries and the URL preset targets from it. The same nine labels had been written out in three places Simplify - NavigationMenuToggle composes NavigationMenuItem rather than repeating its markup and key handling, so a row reacts to input in one place only - ViewParamsSection consulted collapsible five times in a render. Absence of a toggle handler now represents a section which cannot collapse, and the header owns its two shapes - Share the small uppercase group heading through an eyebrow-label mixin, which was copied across four stylesheets Tests - Cover that a collapsed section keeps its values through apply. The form reads from the DOM, so unmounting collapsed fields would silently drop them, and no existing test caught that Also drops a comment claiming rows avoid being buttons because Space is a global hotkey. Measured against the app, the hotkey does not fire while the menu is open regardless of focus, so the rationale did not hold. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014L7R2LgjMSH2WQbiSP4uRn
This commit is contained in:
Claude
@@ -79,9 +79,6 @@
|
||||
flex: 1;
|
||||
overflow-y: auto;
|
||||
padding-block: 0.5rem 1rem;
|
||||
|
||||
scrollbar-width: thin;
|
||||
scrollbar-color: $gray-1000 transparent;
|
||||
}
|
||||
|
||||
.group {
|
||||
@@ -95,10 +92,7 @@
|
||||
|
||||
.groupLabel {
|
||||
padding: 0.5rem 1.25rem 0.25rem;
|
||||
font-size: $aux-text-size;
|
||||
font-weight: 600;
|
||||
letter-spacing: 0.06em;
|
||||
text-transform: uppercase;
|
||||
@include eyebrow-label;
|
||||
color: $gray-700;
|
||||
}
|
||||
|
||||
|
||||
@@ -64,20 +64,20 @@ function NavigationMenu({ isOpen, onClose }: NavigationMenuProps) {
|
||||
<div className={style.body}>
|
||||
<MenuGroup label='This screen'>
|
||||
{supportsFullscreen && (
|
||||
<NavigationMenuToggle checked={fullscreen} icon={<IoExpand />} label='Fullscreen' onClick={toggle} />
|
||||
<NavigationMenuToggle checked={fullscreen} icon={<IoExpand />} label='Fullscreen' onToggle={toggle} />
|
||||
)}
|
||||
<NavigationMenuToggle
|
||||
checked={mirror}
|
||||
icon={<IoSwapVertical />}
|
||||
label='Flip Screen'
|
||||
onClick={() => toggleMirror()}
|
||||
onToggle={() => toggleMirror()}
|
||||
/>
|
||||
{canUseWakeLock && (
|
||||
<NavigationMenuToggle
|
||||
checked={keepAwake}
|
||||
icon={<LuCoffee />}
|
||||
label='Keep Awake'
|
||||
onClick={toggleKeepAwake}
|
||||
onToggle={toggleKeepAwake}
|
||||
/>
|
||||
)}
|
||||
<NavigationMenuItem onClick={handlers.open}>
|
||||
|
||||
+5
-35
@@ -45,6 +45,11 @@
|
||||
background-color: $white-3;
|
||||
}
|
||||
|
||||
// toggle rows are labels, the focusable control sits inside them
|
||||
&:focus-within {
|
||||
background-color: $white-3;
|
||||
}
|
||||
|
||||
&.current {
|
||||
background-color: rgba($blue-500, 0.16);
|
||||
border-left-color: $blue-400;
|
||||
@@ -62,38 +67,3 @@
|
||||
min-width: 0;
|
||||
@include ellipsis-text;
|
||||
}
|
||||
|
||||
/**
|
||||
* Presentational switch for toggle rows.
|
||||
* The row itself owns the interaction (role=switch), so this is aria-hidden:
|
||||
* nesting a real control inside a clickable row would double up the hit target.
|
||||
* Metrics and colours are kept in sync with the medium Switch component.
|
||||
*/
|
||||
.toggleTrack {
|
||||
flex-shrink: 0;
|
||||
box-sizing: content-box;
|
||||
width: 2.5rem;
|
||||
height: calc(1.5rem - 4px);
|
||||
padding: 2px;
|
||||
border-radius: $component-border-radius-full;
|
||||
background-color: $gray-1100;
|
||||
transition: background-color 125ms cubic-bezier(0.26, 0.75, 0.38, 0.45);
|
||||
|
||||
&::after {
|
||||
content: '';
|
||||
display: block;
|
||||
width: calc(1.5rem - 4px);
|
||||
height: 100%;
|
||||
border-radius: $component-border-radius-full;
|
||||
background-color: $ui-white;
|
||||
transition: translate 150ms ease;
|
||||
}
|
||||
}
|
||||
|
||||
[aria-checked='true'] > .toggleTrack {
|
||||
background-color: $blue-700;
|
||||
|
||||
&::after {
|
||||
translate: calc(2.5rem - (1.5rem - 4px)) 0;
|
||||
}
|
||||
}
|
||||
|
||||
+1
@@ -11,6 +11,7 @@ interface NavigationMenuItemProps {
|
||||
onClick: () => void;
|
||||
}
|
||||
|
||||
/** A row in the navigation menu, and the single place which decides how a row reacts to input */
|
||||
export default function NavigationMenuItem({
|
||||
active,
|
||||
className,
|
||||
|
||||
+7
-22
@@ -1,6 +1,6 @@
|
||||
import { ReactNode } from 'react';
|
||||
|
||||
import { isKeyEnter } from '../../../utils/keyEvent';
|
||||
import Switch from '../../switch/Switch';
|
||||
|
||||
import style from './NavigationMenuItem.module.scss';
|
||||
|
||||
@@ -8,31 +8,16 @@ interface NavigationMenuToggleProps {
|
||||
checked: boolean;
|
||||
icon: ReactNode;
|
||||
label: string;
|
||||
onClick: () => void;
|
||||
onToggle: () => void;
|
||||
}
|
||||
|
||||
/**
|
||||
* A menu row which reflects an on/off state.
|
||||
* We keep the div + Enter handling of NavigationMenuItem instead of a button:
|
||||
* Space is a global hotkey for toggling the menu, and a button would react to it as well.
|
||||
*/
|
||||
export default function NavigationMenuToggle({ checked, icon, label, onClick }: NavigationMenuToggleProps) {
|
||||
/** A menu row which reflects, and toggles, an on/off state */
|
||||
export default function NavigationMenuToggle({ checked, icon, label, onToggle }: NavigationMenuToggleProps) {
|
||||
return (
|
||||
<div
|
||||
className={style.link}
|
||||
tabIndex={0}
|
||||
role='switch'
|
||||
aria-checked={checked}
|
||||
onClick={onClick}
|
||||
onKeyDown={(event) => {
|
||||
if (isKeyEnter(event)) {
|
||||
onClick();
|
||||
}
|
||||
}}
|
||||
>
|
||||
<label className={style.link}>
|
||||
{icon}
|
||||
<span className={style.label}>{label}</span>
|
||||
<span className={style.toggleTrack} aria-hidden />
|
||||
</div>
|
||||
<Switch checked={checked} onCheckedChange={onToggle} />
|
||||
</label>
|
||||
);
|
||||
}
|
||||
|
||||
+1
-4
@@ -1,9 +1,6 @@
|
||||
.header {
|
||||
padding: 0 1.25rem 0.25rem;
|
||||
font-size: $aux-text-size;
|
||||
font-weight: 600;
|
||||
letter-spacing: 0.06em;
|
||||
text-transform: uppercase;
|
||||
@include eyebrow-label;
|
||||
color: $gray-700;
|
||||
}
|
||||
|
||||
|
||||
@@ -79,9 +79,6 @@
|
||||
flex: 1;
|
||||
padding: 1rem;
|
||||
overflow-y: auto;
|
||||
|
||||
scrollbar-width: thin;
|
||||
scrollbar-color: $gray-1000 transparent;
|
||||
}
|
||||
|
||||
.footer {
|
||||
|
||||
@@ -4,6 +4,7 @@ import { FormEvent, memo } from 'react';
|
||||
import { IoClose } from 'react-icons/io5';
|
||||
import { useSearchParams } from 'react-router';
|
||||
|
||||
import { viewLabels } from '../../../viewerConfig';
|
||||
import useViewSettings from '../../hooks-query/useViewSettings';
|
||||
import { useIsSmallScreen } from '../../hooks/useIsSmallScreen';
|
||||
import { useSavedViewParams } from '../../stores/savedViewParams';
|
||||
@@ -18,19 +19,6 @@ import ViewParamsSection from './ViewParamsSection';
|
||||
|
||||
import style from './ViewParamsEditor.module.scss';
|
||||
|
||||
/** User facing names for the views we can customise */
|
||||
const viewLabels: Record<OntimeView, string> = {
|
||||
[OntimeView.Editor]: 'Editor',
|
||||
[OntimeView.Cuesheet]: 'Cuesheet',
|
||||
[OntimeView.Operator]: 'Operator',
|
||||
[OntimeView.Timer]: 'Timer',
|
||||
[OntimeView.Backstage]: 'Backstage',
|
||||
[OntimeView.Timeline]: 'Timeline',
|
||||
[OntimeView.StudioClock]: 'Studio Clock',
|
||||
[OntimeView.Countdown]: 'Countdown',
|
||||
[OntimeView.ProjectInfo]: 'Project Info',
|
||||
};
|
||||
|
||||
interface EditFormDrawerProps {
|
||||
target: OntimeView;
|
||||
viewOptions: ViewOption[];
|
||||
|
||||
@@ -11,10 +11,7 @@
|
||||
display: flex;
|
||||
align-items: center;
|
||||
|
||||
font-size: $aux-text-size;
|
||||
font-weight: 600;
|
||||
letter-spacing: 0.06em;
|
||||
text-transform: uppercase;
|
||||
@include eyebrow-label;
|
||||
color: $gray-300;
|
||||
}
|
||||
|
||||
@@ -24,8 +21,6 @@
|
||||
max-height: 10rem;
|
||||
overflow-y: auto;
|
||||
scrollbar-gutter: stable;
|
||||
scrollbar-width: thin;
|
||||
scrollbar-color: $gray-1000 transparent;
|
||||
}
|
||||
|
||||
.preset {
|
||||
|
||||
@@ -14,10 +14,7 @@
|
||||
min-height: 2.75rem;
|
||||
padding: 0.5rem 1rem;
|
||||
|
||||
font-size: $aux-text-size;
|
||||
font-weight: 600;
|
||||
letter-spacing: 0.06em;
|
||||
text-transform: uppercase;
|
||||
@include eyebrow-label;
|
||||
color: $gray-300;
|
||||
|
||||
&.collapsible {
|
||||
|
||||
@@ -30,56 +30,66 @@ export default function ViewParamsSection({ title, collapsible, options }: ViewP
|
||||
return <HiddenContents options={options} />;
|
||||
}
|
||||
|
||||
const handleCollapse = () => {
|
||||
if (collapsible) {
|
||||
setCollapsed((prev) => !prev);
|
||||
}
|
||||
};
|
||||
|
||||
const isCollapsed = Boolean(collapsible && collapsed);
|
||||
const isCustomised = options.some((option) => searchParams.has(option.id));
|
||||
|
||||
return (
|
||||
<section className={style.section}>
|
||||
<div
|
||||
className={cx([style.sectionHeader, collapsible && style.collapsible])}
|
||||
onClick={handleCollapse}
|
||||
onKeyDown={(event) => {
|
||||
if (isKeyEnter(event)) {
|
||||
handleCollapse();
|
||||
}
|
||||
}}
|
||||
role={collapsible ? 'button' : undefined}
|
||||
tabIndex={collapsible ? 0 : undefined}
|
||||
aria-expanded={collapsible ? !isCollapsed : undefined}
|
||||
>
|
||||
<span className={style.sectionTitle}>
|
||||
{title}
|
||||
{isCustomised && <span className={style.customised} title='Contains custom values' />}
|
||||
</span>
|
||||
{collapsible && <IoChevronDown className={cx([style.chevron, isCollapsed && style.closed])} />}
|
||||
</div>
|
||||
<SectionHeader
|
||||
title={title}
|
||||
isCustomised={options.some((option) => searchParams.has(option.id))}
|
||||
collapsed={isCollapsed}
|
||||
onToggle={collapsible ? () => setCollapsed((prev) => !prev) : undefined}
|
||||
/>
|
||||
{/* collapsed options stay mounted: the form reads its values from the DOM */}
|
||||
<div className={cx([style.options, isCollapsed && style.hidden])}>
|
||||
<SectionContents options={options} />
|
||||
</div>
|
||||
</section>
|
||||
);
|
||||
}
|
||||
|
||||
function SectionContents({ options }: { options: ParamField[] }) {
|
||||
return (
|
||||
<>
|
||||
{options.map((option) => {
|
||||
return (
|
||||
{options.map((option) => (
|
||||
<label key={option.title} className={cx([style.label, isInlineField(option) && style.inline])}>
|
||||
<span className={style.title}>{option.title}</span>
|
||||
<span className={style.description}>{option.description}</span>
|
||||
<ParamInput paramField={option} />
|
||||
</label>
|
||||
);
|
||||
})}
|
||||
</>
|
||||
))}
|
||||
</div>
|
||||
</section>
|
||||
);
|
||||
}
|
||||
|
||||
interface SectionHeaderProps {
|
||||
title: string;
|
||||
isCustomised: boolean;
|
||||
collapsed: boolean;
|
||||
/** a header without a toggle belongs to a section which cannot collapse */
|
||||
onToggle?: () => void;
|
||||
}
|
||||
|
||||
function SectionHeader({ title, isCustomised, collapsed, onToggle }: SectionHeaderProps) {
|
||||
const label = (
|
||||
<span className={style.sectionTitle}>
|
||||
{title}
|
||||
{isCustomised && <span className={style.customised} title='Contains custom values' />}
|
||||
</span>
|
||||
);
|
||||
|
||||
if (!onToggle) {
|
||||
return <div className={style.sectionHeader}>{label}</div>;
|
||||
}
|
||||
|
||||
return (
|
||||
<div
|
||||
className={cx([style.sectionHeader, style.collapsible])}
|
||||
role='button'
|
||||
tabIndex={0}
|
||||
aria-expanded={!collapsed}
|
||||
onClick={onToggle}
|
||||
onKeyDown={(event) => {
|
||||
if (isKeyEnter(event)) {
|
||||
onToggle();
|
||||
}
|
||||
}}
|
||||
>
|
||||
{label}
|
||||
<IoChevronDown className={cx([style.chevron, collapsed && style.closed])} />
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
+15
-9
@@ -12,22 +12,28 @@ import { useUpdateUrlPreset } from '../../../../../common/hooks-query/useUrlPres
|
||||
import { isUrlSafe } from '../../../../../common/utils/regex';
|
||||
import { enDash } from '../../../../../common/utils/styleUtils';
|
||||
import { generateUrlPresetOptions } from '../../../../../common/utils/urlPresets';
|
||||
import { viewLabels } from '../../../../../viewerConfig';
|
||||
import CuesheetLinkOptions, { CuesheetPermissionValues } from '../../../../sharing/composite/CuesheetLinkOptions';
|
||||
import * as Panel from '../../../panel-utils/PanelUtils';
|
||||
|
||||
import style from './URLPresetForm.module.scss';
|
||||
|
||||
const targetOptions: SelectOption<OntimeViewPresettable>[] = [
|
||||
{ value: OntimeView.Cuesheet, label: 'Cuesheet' },
|
||||
{ value: OntimeView.Operator, label: 'Operator' },
|
||||
{ value: OntimeView.Timer, label: 'Timer' },
|
||||
{ value: OntimeView.Backstage, label: 'Backstage' },
|
||||
{ value: OntimeView.Timeline, label: 'Timeline' },
|
||||
{ value: OntimeView.StudioClock, label: 'Studio Clock' },
|
||||
{ value: OntimeView.Countdown, label: 'Countdown' },
|
||||
{ value: OntimeView.ProjectInfo, label: 'Project Info' },
|
||||
const presettableTargets: OntimeViewPresettable[] = [
|
||||
OntimeView.Cuesheet,
|
||||
OntimeView.Operator,
|
||||
OntimeView.Timer,
|
||||
OntimeView.Backstage,
|
||||
OntimeView.Timeline,
|
||||
OntimeView.StudioClock,
|
||||
OntimeView.Countdown,
|
||||
OntimeView.ProjectInfo,
|
||||
];
|
||||
|
||||
const targetOptions: SelectOption<OntimeViewPresettable>[] = presettableTargets.map((value) => ({
|
||||
value,
|
||||
label: viewLabels[value],
|
||||
}));
|
||||
|
||||
const formId = 'url-preset-form';
|
||||
|
||||
const defaultValues: URLPreset = {
|
||||
|
||||
@@ -8,6 +8,14 @@
|
||||
text-overflow: ellipsis;
|
||||
}
|
||||
|
||||
/** Small uppercase heading used to label groups of items in drawers and lists */
|
||||
@mixin eyebrow-label {
|
||||
font-size: $aux-text-size;
|
||||
font-weight: 600;
|
||||
letter-spacing: 0.06em;
|
||||
text-transform: uppercase;
|
||||
}
|
||||
|
||||
@mixin rotate-fourty-five {
|
||||
transform: rotate(45deg);
|
||||
}
|
||||
|
||||
@@ -1,12 +1,30 @@
|
||||
export const navigatorConstants = [
|
||||
{ url: 'timer', label: 'Timer' },
|
||||
{ url: 'backstage', label: 'Backstage' },
|
||||
{ url: 'timeline', label: 'Timeline' },
|
||||
{ url: 'studio', label: 'Studio Clock' },
|
||||
{ url: 'countdown', label: 'Countdown' },
|
||||
{ url: 'info', label: 'Project Info' },
|
||||
import { OntimeView } from 'ontime-types';
|
||||
|
||||
/** User facing name of each view, the single source for view naming in the UI */
|
||||
export const viewLabels: Record<OntimeView, string> = {
|
||||
[OntimeView.Editor]: 'Editor',
|
||||
[OntimeView.Cuesheet]: 'Cuesheet',
|
||||
[OntimeView.Operator]: 'Operator',
|
||||
[OntimeView.Timer]: 'Timer',
|
||||
[OntimeView.Backstage]: 'Backstage',
|
||||
[OntimeView.Timeline]: 'Timeline',
|
||||
[OntimeView.StudioClock]: 'Studio Clock',
|
||||
[OntimeView.Countdown]: 'Countdown',
|
||||
[OntimeView.ProjectInfo]: 'Project Info',
|
||||
};
|
||||
|
||||
/** Views offered in the navigation menu, in the order they are shown. Their value is also their route */
|
||||
const navigatorViews = [
|
||||
OntimeView.Timer,
|
||||
OntimeView.Backstage,
|
||||
OntimeView.Timeline,
|
||||
OntimeView.StudioClock,
|
||||
OntimeView.Countdown,
|
||||
OntimeView.ProjectInfo,
|
||||
];
|
||||
|
||||
export const navigatorConstants = navigatorViews.map((view) => ({ url: view, label: viewLabels[view] }));
|
||||
|
||||
// default time format to use for users in 12 hour clocks
|
||||
export const FORMAT_12 = 'h:mm:ss a';
|
||||
// default time format to use for users in 24 hour clocks
|
||||
|
||||
@@ -13,3 +13,26 @@ test('View params configures timer view', async ({ page }) => {
|
||||
await expect(page.getByText('TIME NOW', { exact: true })).not.toBeInViewport();
|
||||
await expect(page).toHaveURL(/.*hideClock=true/);
|
||||
});
|
||||
|
||||
/**
|
||||
* The form gathers its values from the DOM, so a collapsed section has to stay mounted.
|
||||
* Unmounting it would quietly drop everything the user set in it on the next apply.
|
||||
*/
|
||||
test('View params keeps the values of collapsed sections', async ({ page }) => {
|
||||
// hideClock lives in the section we are about to collapse
|
||||
await page.goto('/timer?hideClock=true');
|
||||
await expect(page.getByText('TIME NOW', { exact: true })).not.toBeInViewport();
|
||||
|
||||
await page.mouse.move(Math.random() * 100, Math.random() * 100);
|
||||
await page.getByTestId('navigation__toggle-settings').click();
|
||||
|
||||
const section = page.getByRole('button', { name: /Element visibility/ });
|
||||
await expect(section).toHaveAttribute('aria-expanded', 'true');
|
||||
await section.click();
|
||||
await expect(section).toHaveAttribute('aria-expanded', 'false');
|
||||
|
||||
await page.getByTestId('apply-view-params').click();
|
||||
|
||||
await expect(page).toHaveURL(/.*hideClock=true/);
|
||||
await expect(page.getByText('TIME NOW', { exact: true })).not.toBeInViewport();
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user