rt/wrolpi
1
0
Fork 0
mirror of https://github.com/lrnselfreliance/wrolpi.git synced 2026-10-26 19:01:39 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions

ui: act on review -- corner chrome, a swallowed className, and the poster frame

Browse source 
Three real defects from the PR review, each reproduced before it was fixed.

The tag mark and a video's duration were pinned to the poster BAND, which
is the full width of the card, rather than to the painted image.  Once the
height cap landed the two stopped being the same box: measured, the mark
sat 39px clear of a book cover, 56px clear of a small poster and 60px
clear of a vertical video, and the duration 55px clear.  It went unnoticed
because on a 16:9 poster the image fills the band and the two pin to the
same place -- so the tests cover the height-bound ratios, and assert the
letterbox gap is really there before measuring into it.

CardPoster now centres a frame that shrink-wraps the image, and the frame
is what corner chrome is positioned against.  VideoCard passes its
duration through a new `overlay` prop rather than wrapping CardPoster in a
box of its own.

The mark also went back INSIDE the link.  Splitting it out made the
tagged corner a dead spot on an otherwise clickable poster.

CardLink and ExternalCardLink wrote `className='...' {...props}`, so a
caller's className replaced the pair instead of adding to it.  Every
archive title passes `card-title-ellipsis`, so every archive title lost
`card-link` -- the one card type this PR migrates for layout was the one
missing the title treatment.  Both compose now.

On the fourth comment, the breadth of the ActionIcon remap: it is
deliberate and the numbers are small.  59 IconButton call sites, 52 of
them at the default size, 7 at `xs` (one in Flasher, three in the
inventory schema editor).  The one library control that stands beside an
input rather than a button is SearchBox's clear, and the remap is exactly
what that scale is for -- it now matches its field to the pixel, where
before it was 28px against a 36px input.  Guarded by a test that fails at
42 vs 36 if the default is reverted.

1078 jest, 158 in ui-layout.cy.js, clean build.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Roland 2026-08-02 11:35:49 -06:00 • committed by lrnselfreliance
parent c581dadae6
commit e36bd85f54
5 changed files with 190 additions and 17 deletions
Download patch file Download diff file Expand all files Collapse all files

44
app/src/components/Common.js
View file

@ -174,19 +174,30 @@ export function isoDatetimeToString(dt, time = false) {
return d;
}
export function CardLink({to, newTab = false, ...props}) {
/*
* Both card links COMPOSE the caller's className rather than being overwritten by it.
* Written as `className='...' {...props}`, the spread put the caller's class last and it
* replaced the pair outright -- so every archive title, which passes
* `card-title-ellipsis`, lost `card-link` and rendered in the link colour instead of the
* card's text colour. The one card type whose classes are set this way, and the one whose
* title looked wrong.
*/
const cardLinkClass = (className) =>
['no-link-underscore', 'card-link', className].filter(Boolean).join(' ');
export function CardLink({to, newTab = false, className, ...props}) {
props = newTab === true ? {...props, target: '_blank', rel: 'noopener noreferrer'} : props;
return <Link to={to} className="no-link-underscore card-link" {...props}>
return <Link to={to} className={cardLinkClass(className)} {...props}>
{props.children}
</Link>
}
export function ExternalCardLink({to, children, ...props}) {
export function ExternalCardLink({to, children, className, ...props}) {
return <a
href={to}
target='_blank'
rel='noopener noreferrer'
className='no-link-underscore card-link'
className={cardLinkClass(className)}
{...props}
>
{children}
@ -726,7 +737,7 @@ export function findPosterPath(file) {
}
}
export function CardPoster({to, file}) {
export function CardPoster({to, file, overlay}) {
const navigate = useNavigate();
// Marks a tagged file, pinned to the poster's corner.
@ -739,16 +750,29 @@ export function CardPoster({to, file}) {
// FileGroup has a poster (screenshot/thumbnail) file.
posterPath = `/media/${encodeMediaPath(posterPath)}`;
// Sized by `.wrolpi-card-poster`: as wide as the card allows, natural ratio.
const image = <img alt='poster' src={posterPath}/>;
/*
* Sized by `.wrolpi-card-poster`: as wide as the card allows, capped in height,
* natural ratio. The corner chrome goes INSIDE the link, beside the image:
*
* - the frame shrink-wraps the painted image, so a mark pinned to its corner
* tracks the poster rather than the letterbox space beside it. A height-capped
* poster is a good deal narrower than its band -- 155px inside a 233px card for
* a book cover, less for a vertical video;
* - and the mark stays part of the poster's hit target, rather than a dead spot
* on the corner of an otherwise clickable image.
*/
const inner = <>
{imageLabel}
<img alt='poster' src={posterPath}/>
{overlay}
</>;
return <div className='wrolpi-card-poster'>
{imageLabel}
{to
// Link within this App.
? <Link to={to}>{image}</Link>
? <Link to={to} className='wrolpi-card-poster-frame'>{inner}</Link>
// Preview the file.
: <PreviewLink file={file}>{image}</PreviewLink>}
: <PreviewLink file={file} className='wrolpi-card-poster-frame'>{inner}</PreviewLink>}
</div>
} else {
// FileGroup has no poster.

25
app/src/components/Common.test.js
View file

@ -2,7 +2,8 @@ import React from 'react';
import {act, render, screen, waitFor} from '../test-utils';
import userEvent from '@testing-library/user-event';
import {
contrastingColor, contrastRatio, DirectorySearch, HelpHeader, InfoHeader, loadRole,
CardLink, contrastingColor, contrastRatio, DirectorySearch, ExternalCardLink, HelpHeader,
InfoHeader, loadRole,
SearchResultsInput, TAG_TEXT_DARK, TAG_TEXT_LIGHT,
} from './Common';
import {healthRole} from './admin/ControllerPage';
@ -684,3 +685,25 @@ describe('a heading keeps its help icon on the same row as the text', () => {
expect(container.querySelector('.inline-header')).toBeNull();
});
});
describe('card links', () => {
/*
* `.card-link` is what makes a card's title read as the card's text rather than as a
* link, so a title that misses it is the one thing on the card in the wrong colour.
* Both helpers set the class and then spread the caller's props over it, so any call
* site passing a className of its own -- which is every archive title, via
* `card-title-ellipsis` -- silently replaced the class instead of adding to it.
*/
it.each([
['CardLink', <CardLink to='/archives/1' className='card-title-ellipsis'>Title</CardLink>],
['ExternalCardLink',
<ExternalCardLink to='http://x/a.html' className='card-title-ellipsis'>Title</ExternalCardLink>],
])('%s keeps its own classes when the call site adds one', (_name, element) => {
render(element);
const link = screen.getByText('Title');
expect(link).toHaveClass('card-link');
expect(link).toHaveClass('no-link-underscore');
expect(link).toHaveClass('card-title-ellipsis');
});
});

12
app/src/components/Videos.js
View file

@ -841,10 +841,14 @@ export function VideoCard({file}) {
channel_url = `/videos/channel/${channel.id}/video`;
}
const media = <div style={{position: 'relative'}}>
<CardPoster to={video_url} file={file}/>
<Duration totalSeconds={file.length}/>
</div>;
/*
* The duration goes through CardPoster's `overlay` rather than being pinned to a
* wrapper around it: the wrapper is the full width of the card, and a vertical video's
* poster is only about half that once the height cap applies, so the badge landed well
* clear of the frame it belongs to.
*/
const media = <CardPoster to={video_url} file={file}
overlay={<Duration totalSeconds={file.length}/>}/>;
const title = file.title || file.name || video.stem || video.name;
let titleElm = <Tooltip label={title}>

106
app/src/components/ui/ui-layout.cy.js
View file

@ -1,10 +1,11 @@
import React from 'react';
import {
ActionInput, Button, Card, Header, Icon, IconButton, IconStack, Loading, MultiSelect, Panel, PathInput,
Message, Placeholder, Progress, Statistic, StatisticGroup, Status, TabBar, Table,
Message, Placeholder, Progress, SearchBox, Statistic, StatisticGroup, Status, TabBar, Table,
tabClassName, TextInput,
} from './index';
import {MemoryRouter} from 'react-router';
import {VideoCard} from '../Videos';
import {CardPoster, contrastingColor, HelpHeader, LoadStatistic} from '../Common';
import {Notifications} from '@mantine/notifications';
import {clearToasts, toast} from './toast';
@ -1916,3 +1917,106 @@ describe('a button and an icon button are the same height', () => {
.to.be.at.least(34));
});
});
describe('corner chrome sits on the poster, not on the band around it', () => {
/*
* The tag mark and the video duration are pinned to a corner of "the poster". Once the
* poster gained a height cap, the painted image stopped filling the band it sits in: a
* portrait cover is 155px wide inside a 233px card, and a vertical video narrower still.
* The mark then floats in empty letterbox space nowhere near the image's corner.
*
* Geometry against a real painted image, so it belongs here. It has to be checked on a
* ratio that is HEIGHT-bound -- on a 16:9 poster the image fills the band and a mark
* pinned to either one lands in the same place, which is why this went unnoticed.
*/
const tagged = (width, height) => ({...posterFile(width, height), tags: [{name: 'ham radio'}]});
[
{name: 'a portrait book cover', w: 306, h: 396},
{name: 'a vertical video', w: 720, h: 1280},
{name: 'a poster smaller than the card', w: 120, h: 90},
].forEach(({name, w, h}) => {
it(`pins the tag to the corner of ${name}`, () => {
servePoster(w, h);
cy.mountUI(narrowCard(<CardPoster file={tagged(w, h)}/>));
cy.get('.wrolpi-card-poster img')
.should(($img) => expect($img[0].naturalWidth, 'poster loaded').to.equal(w))
.should(($img) => {
const image = $img[0].getBoundingClientRect();
const band = $img[0].closest('.wrolpi-card-poster').getBoundingClientRect();
// The premise: this ratio really does leave letterbox space to get lost in.
expect(band.width - image.width, `${name} is narrower than its band`)
.to.be.greaterThan(20);
const mark = Cypress.$('.wrolpi-card-tag')[0].getBoundingClientRect();
expect(mark.left, 'tag tracks the image, not the band')
.to.be.closeTo(image.left, 2);
expect(mark.top, 'tag sits at the top of the image').to.be.closeTo(image.top, 2);
});
});
});
it('keeps the tag inside the poster link, so clicking it still navigates', () => {
// The mark used to render inside the Link beside the image and was part of the
// poster's hit target; a sibling placed before the link is a dead spot on the corner.
servePoster(306, 396);
cy.mountUI(narrowCard(<CardPoster to='/videos/1' file={tagged(306, 396)}/>));
cy.get('.wrolpi-card-tag').should(($tag) =>
expect($tag[0].closest('a'), 'the tag is inside the link').to.not.be.null);
});
});
describe('a video card pins its duration to the poster', () => {
/*
* The same defect as the tag mark, on the other corner and reached by a different
* route: the duration was pinned to a wrapper around CardPoster rather than to the
* poster itself. Mounted through the real VideoCard, because the wrapper was the
* subject and a hand-built poster would not have one.
*/
it('keeps the duration on a vertical video, not out in the band', () => {
servePoster(720, 1280);
// mountWithRouter, not mountUI: VideoCard reads the sort order from the query
// context, which the spartan mount deliberately does not provide.
cy.mountWithRouter(<div style={{width: 233}}><VideoCard file={{
id: 1, tags: [], length: 95, primary_path: 'videos/short.mp4',
title: 'A vertical video', mimetype: 'video/mp4',
poster_path: 'videos/poster-720x1280.svg',
video: {stem: 'short', channel: null},
}}/></div>);
cy.get('.wrolpi-card-poster img')
.should(($img) => expect($img[0].naturalWidth, 'poster loaded').to.equal(720))
.should(($img) => {
const image = $img[0].getBoundingClientRect();
const band = $img[0].closest('.wrolpi-card-poster').getBoundingClientRect();
expect(band.width - image.width, 'the poster is narrower than its band')
.to.be.greaterThan(20);
const badge = Cypress.$('.duration-overlay')[0].getBoundingClientRect();
expect(badge.right, 'the duration tracks the image, not the band')
.to.be.closeTo(image.right, 6);
});
});
});
describe('the search clear button matches the field it clears', () => {
/*
* Aligning IconButton with Button changed every icon button in the app, and this is the
* one in the library that stands beside an input rather than beside a button. It is
* also the case the remap was built for: `--ai-size-input-*` exists precisely so an
* ActionIcon can match an Input, so if this pair is ragged the remap picked the wrong
* scale.
*/
it('is exactly as tall as the search field', () => {
cy.mountUI(<SearchBox placeholder='Search' clearable value='axe' onChange={() => {}}/>);
cy.get('.wrolpi-searchbox-clear').should(($clear) => {
const input = Cypress.$('input')[0].getBoundingClientRect();
const clear = $clear[0].getBoundingClientRect();
expect(input.height, 'the field has a height at all').to.be.greaterThan(10);
expect(clear.height, 'clear button vs search field').to.be.closeTo(input.height, 0.5);
});
});
});

20
app/src/components/ui/ui.css
View file

@ -1154,8 +1154,27 @@ html[data-theme="amber"] .wrolpi-progress-fill {
--card-poster-max-height: 200px;
}
/*
* The band is the full width of the card and centres the poster in it. The frame inside
* shrink-wraps the painted image, and is what corner chrome -- the tagged mark, a video's
* duration -- is positioned against. Pinning that chrome to the BAND puts it in the
* letterbox space beside a height-capped poster: 39px adrift on a book cover, 60px on a
* vertical video.
*
* The frame's own width falls out of the caps below. Its intrinsic contribution is the
* image's natural width already reduced by `max-height`, so a portrait cover contributes
* 155px and gets 155px; a 16:9 poster contributes 1280px and is flex-shrunk to the band,
* at which point `max-width: 100%` resolves against a width that is finally definite.
*/
.wrolpi-card-poster {
display: flex;
justify-content: center;
}
.wrolpi-card-poster-frame {
position: relative;
display: block;
min-width: 0;
}
.wrolpi-card-poster img {
@ -1164,7 +1183,6 @@ html[data-theme="amber"] .wrolpi-progress-fill {
height: auto;
max-width: 100%;
max-height: var(--card-poster-max-height);
margin: 0 auto;
}
.wrolpi-card-icon {