diff --git a/app/src/App.css b/app/src/App.css index cbd5fb86..094e7696 100644 --- a/app/src/App.css +++ b/app/src/App.css @@ -82,8 +82,14 @@ pre.wrap-text { text-decoration: none !important; } +/* + * A card's title and its author/channel/domain line are links, but they are the card's + * text -- they take the body colour rather than the link colour, so a grid of cards reads + * as titles and not as a wall of blue. This was a literal `black` until the themes + * arrived, which left every title at about 1.1:1 on the dark, night and amber panels. + */ .card-link { - color: black; + color: var(--text); } .center-me { diff --git a/app/src/components/Archive.js b/app/src/components/Archive.js index a8d97418..ec49bfe1 100644 --- a/app/src/components/Archive.js +++ b/app/src/components/Archive.js @@ -426,22 +426,22 @@ export function ArchiveCard({file}) { ; - return -
- - - - -
- + const actions =
+ + + + +
; + + return } // Domain table column configuration diff --git a/app/src/components/Common.js b/app/src/components/Common.js index 9ec33092..566a6de2 100644 --- a/app/src/components/Common.js +++ b/app/src/components/Common.js @@ -727,8 +727,6 @@ export function findPosterPath(file) { } export function CardPoster({to, file}) { - // Used to center posters in CardIcon. - const style = {display: 'flex', justifyContent: 'center'}; const navigate = useNavigate(); // Marks a tagged file, pinned to the poster's corner. @@ -741,25 +739,17 @@ export function CardPoster({to, file}) { // FileGroup has a poster (screenshot/thumbnail) file. posterPath = `/media/${encodeMediaPath(posterPath)}`; - const image = <> - {/* Replicate but with maxHeight applied to image */} - {imageLabel} - poster - ; + // Sized by `.wrolpi-card-poster`: as wide as the card allows, natural ratio. + const image = poster; - if (to) { - // Link within this App. - return - {image} - - } else { - // Preview the file. - return
- - {image} - -
- } + return
+ {imageLabel} + {to + // Link within this App. + ? {image} + // Preview the file. + : {image}} +
} else { // FileGroup has no poster. if (!to || (to.startsWith('/media/') || to.startsWith('/download/'))) { diff --git a/app/src/components/ThemeSamplePage.js b/app/src/components/ThemeSamplePage.js index 40e55625..e022e63c 100644 --- a/app/src/components/ThemeSamplePage.js +++ b/app/src/components/ThemeSamplePage.js @@ -54,6 +54,15 @@ import {contrastingColor} from './Common'; * already happened more than once. */ +/* + * A stand-in poster of exact intrinsic dimensions, so the poster caps can be seen against + * the ratios that actually break them -- a real library has all five and the gallery has + * no media of its own. + */ +const samplePoster = (width, height) => `data:image/svg+xml,${encodeURIComponent( + `` + + ``)}`; + const Section = ({label, children}) =>

} />)} + +

Posters
+ + {[ + ['16:9 video thumbnail', 1280, 720], + ['4:3 screenshot', 800, 600], + ['square thumbnail', 600, 600], + ['portrait book cover', 306, 396], + ['smaller than the card', 120, 90], + ].map(([label, w, h]) => + + } + />)} + +

+ A poster is capped on both axes — the card's width, and{' '} + --card-poster-max-height — and keeps its own ratio whichever + cap bites. Capping the width alone let a square thumbnail stand as tall as + the card is wide. A poster smaller than the card is left at its own size + rather than upscaled. +

+ +
Meta, body and actions
+ + {[ + ['Rainwater Harvesting And Filtration For A Small Off-Grid Homestead', + 'engineering775.com · 2025-01-30', true], + ['Solar Basics', 'wiki.example.org · 2024-08-02', true], + ['Field Notes.pdf', 'docs/ · 2023-06-14', false], + ].map(([title, meta, withActions]) => {title}} + meta={meta} + color='blue' + actions={withActions ?
+ + +
: undefined} + />)} + +

+ Titles are links but take the body colour, not the link colour — a grid of + results should read as titles. The meta line sits directly under the title; + actions are pushed to the foot, so a row lines its buttons up however many + lines each title took. A card with no actions simply ends. +

The bottom edge carries the file type's colour, so a grid of results is scannable by kind before any title is read. It comes from a token, so night diff --git a/app/src/components/ui/Surfaces.tsx b/app/src/components/ui/Surfaces.tsx index eb62239c..40bb6174 100644 --- a/app/src/components/ui/Surfaces.tsx +++ b/app/src/components/ui/Surfaces.tsx @@ -149,8 +149,19 @@ export interface CardProps { /** Image or poster area above the body. */ media?: React.ReactNode; title?: React.ReactNode; + /** + * The card's secondary line -- author, channel, domain, date. It sits directly under + * the title rather than at the foot of the card: a grid stretches every card to the + * tallest in its row, so a foot-anchored meta line strands the date a long way from + * the title it belongs to. + */ meta?: React.ReactNode; children?: React.ReactNode; + /** + * Controls, rendered last and pushed to the card's foot so that a row of cards lines + * its buttons up regardless of how many lines each title took. + */ + actions?: React.ReactNode; onClick?: React.MouseEventHandler; /** * A token colour name, drawn as an accent along the card's top edge. File cards use @@ -162,7 +173,7 @@ export interface CardProps { className?: string; } -export function Card({media, title, meta, children, onClick, color, className}: CardProps) { +export function Card({media, title, meta, children, actions, onClick, color, className}: CardProps) { return {media}}

{title &&
{title}
} + {meta &&
{meta}
} {children} - {meta &&
{meta}
} + {actions &&
+ {actions} +
}
} diff --git a/app/src/components/ui/ui-layout.cy.js b/app/src/components/ui/ui-layout.cy.js index 35175ca6..cbbde7d9 100644 --- a/app/src/components/ui/ui-layout.cy.js +++ b/app/src/components/ui/ui-layout.cy.js @@ -1,10 +1,11 @@ import React from 'react'; import { - ActionInput, Button, Header, Icon, IconStack, Loading, MultiSelect, Panel, PathInput, + ActionInput, Button, Card, Header, Icon, IconStack, Loading, MultiSelect, Panel, PathInput, Message, Placeholder, Progress, Statistic, StatisticGroup, Status, TabBar, Table, tabClassName, TextInput, } from './index'; -import {contrastingColor, HelpHeader, LoadStatistic} from '../Common'; +import {MemoryRouter} from 'react-router'; +import {CardPoster, contrastingColor, HelpHeader, LoadStatistic} from '../Common'; import {Notifications} from '@mantine/notifications'; import {clearToasts, toast} from './toast'; import {monochromeThemes, themeNames} from '../../themes/names'; @@ -1672,3 +1673,193 @@ describe('media filtering', () => { expect(getComputedStyle($img[0]).filter).to.equal('none')); }); }); + +/* + * A poster of exact intrinsic dimensions. It has to come over the wire rather than as a + * data: URI, because CardPoster rewrites whatever it is given into `/media/` -- so + * the bytes are served by an intercept and the component builds its own URL, which is the + * URL the app actually requests. + */ +const posterFile = (width, height) => ({ + id: 1, + tags: [], + poster_path: `videos/poster-${width}x${height}.svg`, + primary_path: 'videos/one.mp4', +}); + +const servePoster = (width, height) => cy.intercept( + 'GET', `**/media/videos/poster-${width}x${height}.svg`, + { + headers: {'content-type': 'image/svg+xml'}, + body: `` + + ``, + }, +); + +/* A card as narrow as the default grid makes them, which is where the cropping showed. */ +const narrowCard = (children) => +
{children}
; + +/* + * The height cap, read from the stylesheet rather than repeated here, so the two cannot + * drift. Deleting the declaration makes this NaN and every comparison against it fails, + * which is the point -- and the bound below keeps "read whatever the CSS says" from being + * a test that any value at all satisfies. + */ +const cardPosterCap = () => { + const value = parseFloat(getComputedStyle(document.documentElement) + .getPropertyValue('--card-poster-max-height')); + expect(value, 'a poster cap is declared, and is small enough to be a cap') + .to.be.within(80, 240); + return value; +}; + +describe('a card poster is bounded by the card, and keeps its ratio', () => { + /* + * The poster carried a fixed 290x163 box. A card in the default grid is about 233px + * wide and Mantine's Card sets `overflow: hidden`, so the browser painted the middle + * 233px of a 290px image: the ratio was right in the layout tree and wrong on screen. + * None of that is visible to jsdom, which lays nothing out. + * + * The cap that replaced it is on the WIDTH, so a square or portrait source grows as + * tall as it likes -- a 1:1 thumbnail became a 233px-tall band above a two-line title. + * Hence a height cap as well. Both caps have to preserve the ratio, which is the part + * worth testing: `max-width` and `max-height` together only "contain" an image while + * its own width and height stay `auto`. + */ + const cases = [ + {name: 'a 16:9 video thumbnail', w: 1280, h: 720}, + {name: 'a square thumbnail', w: 600, h: 600}, + {name: 'a portrait book cover', w: 306, h: 396}, + {name: 'a poster smaller than the card', w: 120, h: 90}, + ]; + + cases.forEach(({name, w, h}) => { + it(`fits ${name} inside the card without distorting it`, () => { + servePoster(w, h); + cy.mountUI(narrowCard()); + + cy.get('.wrolpi-card-poster img') + // The intrinsic size has to have arrived, or every ratio below is 0/0. + .should(($img) => expect($img[0].naturalWidth, 'poster loaded').to.equal(w)) + .should(($img) => { + const img = $img[0]; + const box = img.getBoundingClientRect(); + const card = img.closest('.wrolpi-card-poster').getBoundingClientRect(); + + expect(box.width, 'poster stays within the card').to.be.at.most(card.width + 0.5); + expect(box.height, 'poster is capped in height').to.be.at.most(cardPosterCap() + 0.5); + // Never upscaled: a small poster is left at its own size. + expect(box.width, 'poster is not stretched past its own size').to.be.at.most(w + 0.5); + expect(box.width / box.height, `${name} keeps its ratio`) + .to.be.closeTo(w / h, 0.01); + }); + }); + }); + + it('shrinks a tall poster by its width rather than squashing it', () => { + /* + * The failure mode a ratio check alone would miss: a height cap applied while the + * width is pinned crops or squashes instead of containing. A 600x600 source in a + * 233px card is width-bound at 233 if the cap is loose and height-bound if it is + * tight -- either way the OTHER axis must have moved with it. + */ + servePoster(600, 600); + cy.mountUI(narrowCard()); + + cy.get('.wrolpi-card-poster img') + .should(($img) => expect($img[0].naturalWidth).to.equal(600)) + .should(($img) => { + const box = $img[0].getBoundingClientRect(); + expect(box.height, 'the cap actually bit').to.be.at.most(cardPosterCap() + 0.5); + expect(box.width, 'and the width came down with it') + .to.be.closeTo(box.height, 0.5); + }); + }); +}); + +describe('a card reads as titles, not as a wall of links', () => { + /* + * A card's title and its author/channel/domain line are links, and they carried a + * literal `color: black` from before the themes existed. On the dark, night and amber + * panels that is about 1.1:1 -- the titles were there, and invisible. The date beside + * them was fine, because it takes `--muted` from the card body, which is what made the + * bug look like a card-title bug rather than a link bug. + * + * jsdom cannot see this: `.card-link` now resolves a custom property, and jsdom drops + * `color: var(--text)` as an invalid declaration. + */ + themeNames.forEach((theme) => { + it(`keeps a card title legible in ${theme}`, () => { + cy.mountUI( + + How To Sharpen An Axe + } meta='Wranglerstar'/> + , {theme}); + + cy.get('.card-link').should(($link) => { + const panel = toRgb(getComputedStyle(document.documentElement) + .getPropertyValue('--panel')); + const title = toRgb(getComputedStyle($link[0]).color); + expect(contrast(title, panel), `card title on the panel in ${theme}`) + .to.be.greaterThan(4.5); + }); + }); + }); +}); + +describe('a card keeps its meta with the title and its actions at the foot', () => { + /* + * A grid stretches every card to the tallest in its row. The meta line used to carry + * `margin-top: auto`, so on /docs a PDF with no author had its date shoved 55px below + * the title it belongs to, at the bottom of a card that was tall only because of its + * neighbour. Actions are the thing that wants the foot -- a row of cards should line + * its buttons up however many lines each title took. + * + * Real layout, because `margin-top: auto` in a flex column is resolved by the layout + * engine and jsdom has none. The DOM ORDER half of this is in ui.test.js. + */ + const row =
+ {[ + 'A title long enough to wrap onto three separate lines inside a narrow card', + 'Short', + ].map((title, index) =>
+ Details}/> +
)} +
; + + it('leaves no gap between the title and the meta line', () => { + cy.mountUI(row); + + cy.get('.wrolpi-card-meta').should(($metas) => { + expect($metas.length).to.equal(2); + const gaps = [...$metas].map((meta) => { + const title = meta.previousElementSibling; + return meta.getBoundingClientRect().top - title.getBoundingClientRect().bottom; + }); + // The card body's own 4px gap, and nothing else. + gaps.forEach((gap, index) => expect(gap, `card ${index} title-to-meta`).to.be.at.most(6)); + // Both cards really were stretched to the same height, or nothing was pushed. + const heights = [...Cypress.$('.mantine-Card-root')] + .map(card => Math.round(card.getBoundingClientRect().height)); + expect(new Set(heights).size, 'the row stretched both cards equally').to.equal(1); + }); + }); + + it('lines the actions up at the foot however tall the title got', () => { + cy.mountUI(row); + + cy.get('.wrolpi-card-actions').should(($actions) => { + expect($actions.length).to.equal(2); + const [tall, short] = [...$actions]; + // The premise: the two titles really are different heights. + const titleHeights = [...Cypress.$('.mantine-Card-root')].map(card => + Math.round(card.lastElementChild.firstElementChild.getBoundingClientRect().height)); + expect(new Set(titleHeights).size, 'the two titles differ in height').to.equal(2); + + expect(tall.getBoundingClientRect().bottom, 'both action rows sit on the same line') + .to.be.closeTo(short.getBoundingClientRect().bottom, 1); + }); + }); +}); diff --git a/app/src/components/ui/ui.css b/app/src/components/ui/ui.css index 255d8c7a..b9e1e93e 100644 --- a/app/src/components/ui/ui.css +++ b/app/src/components/ui/ui.css @@ -1117,6 +1117,34 @@ html[data-theme="amber"] .wrolpi-progress-fill { * poster area agree in every theme. */ +/* + * A poster is bounded by the card and keeps whatever ratio the source image has. It used + * to carry a fixed 290x163 box, which is wider than a card in a default grid (~233px at + * desktop widths), so `overflow: hidden` on the card cropped the sides off and the poster + * read as the wrong aspect ratio. + * + * Both caps are needed. Bounding the width alone lets a square thumbnail stand as tall as + * the card is wide, and a portrait book cover taller still, so the media band dwarfs the + * title it belongs to. `width` and `height` stay `auto` so the pair behaves as "contain": + * whichever cap bites first, the other axis comes down with it and the ratio survives. + */ +:root { + --card-poster-max-height: 200px; +} + +.wrolpi-card-poster { + position: relative; +} + +.wrolpi-card-poster img { + display: block; + width: auto; + height: auto; + max-width: 100%; + max-height: var(--card-poster-max-height); + margin: 0 auto; +} + .wrolpi-card-icon { position: relative; display: flex; diff --git a/app/src/components/ui/ui.test.js b/app/src/components/ui/ui.test.js index 1201db4a..c93e6442 100644 --- a/app/src/components/ui/ui.test.js +++ b/app/src/components/ui/ui.test.js @@ -894,6 +894,37 @@ describe('Card', () => { expect(container.firstChild.style.borderBottom).toBe(''); }); + + it('puts the meta line under the title and the actions last', () => { + /* + * Order is the half of the fix jsdom can judge. An Archive card renders its domain + * and date as `meta` and its Details/Open buttons as `actions`; before `actions` + * existed the buttons were children and the meta was foot-anchored, so every archive + * card read buttons-then-domain. Which slot lands where is what this asserts; that + * the foot anchoring actually resolves is in ui-layout.cy.js, since `margin-top: + * auto` needs a layout engine. + */ + renderUI(Details}> + body + ); + + const body = screen.getByText('Rainwater Harvesting').parentElement; + expect([...body.children].map(child => child.textContent)).toEqual([ + 'Rainwater Harvesting', 'engineering775.com', 'body', 'Details', + ]); + expect(body.querySelector('.wrolpi-card-actions')).toHaveStyle({marginTop: 'auto'}); + expect(body.querySelector('.wrolpi-card-meta').style.marginTop).toBe(''); + }); + + it('renders no actions row when a card has no actions', () => { + // Otherwise every card in a grid pays for an empty foot, and the spacing below the + // meta line differs between a card with buttons and one without. + const {container} = renderUI(); + + expect(container.querySelector('.wrolpi-card-actions')).toBeNull(); + }); }); describe('CardGroup', () => {