mirror of
https://github.com/lrnselfreliance/wrolpi.git
synced 2026-10-26 19:01:39 -04:00
ui: give tables a real header, drop the column grid, quiet the row rules
Three things the file browser made obvious.
The header was not a surface. Mantine gives thead no background at all, so
the header painted whatever was behind the table -- which is exactly what an
unstriped body row paints. Against a striped body it read as one more stripe,
and bold type was the only thing distinguishing it. It now carries `--head`,
the token that already means "recessed surface", with a full-strength 2px rule
under it: the header/body division is the one line in a table that carries
meaning, so it is the one allowed to be loud.
Column borders are gone. A line around every cell is a lot of ink to repeat
what the column headings already say, and it was half of why the header
disappeared -- a bordered header cell looks like a cell in a grid.
Row rules were `--border`, the same weight as the outline around the table, so
a long table read as a stack of boxes. They now use `--table-line`, a new
token per theme sitting between the surface and `--border`. The table outline
is not a row rule and keeps `--border`; Mantine draws both from
`--table-border-color`, so that had to be restated.
In the file browser an icon and a filename are inline siblings in one cell. An
inline SVG sits on the text baseline, so a 24px glyph beside 13px text hung
above it -- the icon looked centred in the row and the name looked like it was
resting on the icon's floor. `vertical-align: middle` centres it, and the
margin is a gap that was never there at all: the folder icon touched the first
letter of its own name.
Cypress now loads App.css, as the app does. Without it a spec covering the
file browser rows measures a component the app never renders.
One jest test and eleven browser tests, each checked red against a planted
regression. Two notes for the next person:
- `html .mantine-Table-table` is specificity, not decoration. Mantine
declares --table-border-color on its own hashed class, which ties with
`.mantine-Table-table` and wins on source order.
- the "quieter than the outline" test measures both lines against `--panel`.
Nothing paints <html> under a mounted component, so measuring against the
root background compares them to black, which ranks the *paler* line as the
louder one -- the assertion passed backwards until it was pointed at the
surface the rules actually sit on.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
450aaa7334
commit
eab13e4803
7 changed files with 274 additions and 5 deletions
|
|
@ -22,6 +22,12 @@ import '@mantine/core/styles.css';
|
|||
// App.js loads this too. Without it a Notification renders unstyled here, so a toast had no
|
||||
// surface and no title colour of Mantine's -- and its contrast test could not fail.
|
||||
import '@mantine/notifications/styles.css';
|
||||
/*
|
||||
* App.js loads this as well. Rules for things outside the component library live here --
|
||||
* the nav bar, the file browser rows -- and without it a spec covering one of them measures
|
||||
* a component the app never renders.
|
||||
*/
|
||||
import '../../src/App.css';
|
||||
// react-router v7 ships these directly; `react-router-dom` is not installed.
|
||||
import {MemoryRouter, Route, Routes} from "react-router";
|
||||
import {QueryProvider} from "../../src/hooks/customHooks";
|
||||
|
|
|
|||
|
|
@ -215,6 +215,21 @@ td.file-path {
|
|||
cursor: pointer;
|
||||
}
|
||||
|
||||
/*
|
||||
* The row is an icon followed by a filename, as inline siblings. An inline SVG
|
||||
* sits on the text baseline by default, so a 24px glyph next to 13px text hangs
|
||||
* well above it and the name looks like it is resting on the icon's floor.
|
||||
* `middle` centres the glyph against the text instead.
|
||||
*
|
||||
* The margin is the gap that was never there: the folder icon touched the first
|
||||
* letter of its own name, and two icons (a folder and its ignored marker) touched
|
||||
* each other.
|
||||
*/
|
||||
td.file-path > svg {
|
||||
vertical-align: middle;
|
||||
margin-right: 0.35em;
|
||||
}
|
||||
|
||||
/* Necessary until react-semantic-toasts is upgraded to Node 18 */
|
||||
.ui-alerts {
|
||||
position: fixed;
|
||||
|
|
|
|||
|
|
@ -5,9 +5,15 @@ import {Icon} from './Icon';
|
|||
/*
|
||||
* Tables.
|
||||
*
|
||||
* Borders do structural work here: full 1px cell borders, a `--head` header row,
|
||||
* and zebra striping. Wide tables scroll in their own container so the page
|
||||
* body never scrolls sideways.
|
||||
* Rows are separated by a horizontal rule and nothing else. There are no column
|
||||
* borders: a full grid draws a line around every cell, which is a lot of ink to
|
||||
* say something the column headings already say, and it made the header row --
|
||||
* which had no surface of its own -- read as one more cell in the grid. The
|
||||
* header now carries `--head` and a full-strength rule under it, and the rules
|
||||
* between body rows are `--table-line`, deliberately fainter. See ui.css.
|
||||
*
|
||||
* Wide tables scroll in their own container so the page body never scrolls
|
||||
* sideways.
|
||||
*
|
||||
* The compound names mirror Semantic's (Header/Body/Row/Cell/HeaderCell/Footer)
|
||||
* so migrating a call site is a rename, not a rewrite.
|
||||
|
|
@ -21,7 +27,6 @@ export interface TableProps extends MTableProps {
|
|||
function TableBase({scrollable = true, ...props}: TableProps) {
|
||||
const table = <MTable
|
||||
withTableBorder
|
||||
withColumnBorders
|
||||
striped
|
||||
highlightOnHover={false}
|
||||
verticalSpacing={7}
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
import React from 'react';
|
||||
import {
|
||||
ActionInput, Button, Header, Icon, IconStack, Loading, MultiSelect, Panel, PathInput,
|
||||
Placeholder, Statistic, StatisticGroup, TabBar, tabClassName, TextInput,
|
||||
Placeholder, Statistic, StatisticGroup, TabBar, Table, tabClassName, TextInput,
|
||||
} from './index';
|
||||
import {contrastingColor} from '../Common';
|
||||
import {Notifications} from '@mantine/notifications';
|
||||
|
|
@ -146,6 +146,177 @@ describe('ActionInput', () => {
|
|||
});
|
||||
});
|
||||
|
||||
/* WCAG contrast between two rgb() strings, for judging how loud a border is. */
|
||||
const luminance = (colour) => {
|
||||
const [r, g, b] = (colour.match(/[\d.]+/g) || []).map(Number);
|
||||
const channel = (v) => {
|
||||
const s = v / 255;
|
||||
return s <= 0.03928 ? s / 12.92 : Math.pow((s + 0.055) / 1.055, 2.4);
|
||||
};
|
||||
return 0.2126 * channel(r) + 0.7152 * channel(g) + 0.0722 * channel(b);
|
||||
};
|
||||
|
||||
const contrast = (a, b) => {
|
||||
const [x, y] = [luminance(a), luminance(b)].sort((p, q) => q - p);
|
||||
return (x + 0.05) / (y + 0.05);
|
||||
};
|
||||
|
||||
const sampleTable = <Table>
|
||||
<Table.Header>
|
||||
<Table.Row>
|
||||
<Table.HeaderCell>URL</Table.HeaderCell>
|
||||
<Table.HeaderCell>Size</Table.HeaderCell>
|
||||
</Table.Row>
|
||||
</Table.Header>
|
||||
<Table.Body>
|
||||
<Table.Row><Table.Cell>one</Table.Cell><Table.Cell>1 kB</Table.Cell></Table.Row>
|
||||
<Table.Row><Table.Cell>two</Table.Cell><Table.Cell>2 kB</Table.Cell></Table.Row>
|
||||
<Table.Row><Table.Cell>three</Table.Cell><Table.Cell>3 kB</Table.Cell></Table.Row>
|
||||
</Table.Body>
|
||||
</Table>;
|
||||
|
||||
describe('a table header is a surface, not just bold text', () => {
|
||||
themeNames.forEach((theme) => {
|
||||
it(`separates the header from every body row in ${theme}`, () => {
|
||||
/*
|
||||
* Mantine gives thead no background, so the header painted whatever was behind
|
||||
* the table -- which is exactly what an unstriped body row paints. Against a
|
||||
* striped body the header read as one more stripe, and weight was the only
|
||||
* thing telling them apart. None of this is visible to jsdom: the colours are
|
||||
* tokens, and "what is behind an unstriped row" is a question about painting.
|
||||
*/
|
||||
cy.mountUI(sampleTable, {theme});
|
||||
|
||||
cy.get('thead th').first().should(($th) => {
|
||||
const header = getComputedStyle($th[0]).backgroundColor;
|
||||
expect(header, 'the header paints a surface of its own')
|
||||
.to.not.match(/^rgba\(.*,\s*0(\.0+)?\)$/);
|
||||
|
||||
const rows = [...Cypress.$('tbody tr')].map(row => {
|
||||
// An unstriped row paints nothing, so what shows is the page behind it.
|
||||
const own = getComputedStyle(row).backgroundColor;
|
||||
return /^rgba\(.*,\s*0(\.0+)?\)$/.test(own)
|
||||
? getComputedStyle(document.documentElement).backgroundColor
|
||||
: own;
|
||||
});
|
||||
expect(rows.length).to.be.greaterThan(2);
|
||||
rows.forEach((row, index) => {
|
||||
expect(header, `header vs body row ${index}`).to.not.equal(row);
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
it('rules off the header with a heavier line than it puts between rows', () => {
|
||||
/*
|
||||
* The header/body division is the one that carries meaning -- everything above it
|
||||
* names, everything below is data -- so it is the one line allowed to be loud.
|
||||
*/
|
||||
cy.mountUI(sampleTable, {theme: 'light'});
|
||||
|
||||
cy.get('thead th').first().should(($th) => {
|
||||
const header = getComputedStyle($th[0]);
|
||||
const row = getComputedStyle(Cypress.$('tbody tr')[0]);
|
||||
expect(parseFloat(header.borderBottomWidth), 'header rule width')
|
||||
.to.be.greaterThan(parseFloat(row.borderBottomWidth));
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('table rules', () => {
|
||||
themeNames.forEach((theme) => {
|
||||
it(`keeps the rule between rows quieter than the table outline in ${theme}`, () => {
|
||||
/*
|
||||
* With a rule under every row at full border strength, a long table reads as a
|
||||
* stack of boxes rather than a list. `--table-line` is the quieter token; the
|
||||
* outline around the table is not a row rule and keeps `--border`.
|
||||
*
|
||||
* Measured as contrast against the row's own surface rather than compared to a
|
||||
* token, so the claim is about what the eye gets.
|
||||
*/
|
||||
cy.mountUI(sampleTable, {theme});
|
||||
|
||||
cy.get('tbody tr').first().should(($row) => {
|
||||
const table = Cypress.$('table')[0];
|
||||
const token = (name) => hexToRgb(getComputedStyle(document.documentElement)
|
||||
.getPropertyValue(name).trim());
|
||||
/*
|
||||
* Against `--panel`, the surface a row paints on. Not against the root
|
||||
* background: nothing paints <html> under a mounted component, so it reads
|
||||
* `rgba(0, 0, 0, 0)`, and measuring both lines against black ranks the
|
||||
* *paler* one as louder -- which is how this assertion first passed
|
||||
* backwards.
|
||||
*/
|
||||
const surface = token('--panel');
|
||||
const rule = contrast(getComputedStyle($row[0]).borderBottomColor, surface);
|
||||
const outline = contrast(getComputedStyle(table).borderTopColor, surface);
|
||||
|
||||
expect(rule, 'row rule is quieter than the table outline').to.be.lessThan(outline);
|
||||
});
|
||||
});
|
||||
|
||||
it(`draws nothing between the cells of a row in ${theme}`, () => {
|
||||
cy.mountUI(sampleTable, {theme});
|
||||
|
||||
cy.get('tbody tr').first().find('td').first().should(($cell) => {
|
||||
expect(parseFloat(getComputedStyle($cell[0]).borderRightWidth),
|
||||
'no rule between columns').to.equal(0);
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('a file row lines its icon up with its name', () => {
|
||||
/*
|
||||
* The file browser writes an icon and a filename as inline siblings in one cell. An
|
||||
* inline SVG sits on the text baseline, so a 24px glyph beside 13px text hangs well
|
||||
* above it -- the icon looked centred in the row and the name looked like it was
|
||||
* resting on the icon's floor.
|
||||
*
|
||||
* Purely a question of layout: jsdom returns zeros from getBoundingClientRect, so
|
||||
* nothing there can tell a centred glyph from one hanging off a baseline.
|
||||
*/
|
||||
const fileRow = <Table>
|
||||
<Table.Body>
|
||||
<Table.Row>
|
||||
<Table.Cell className='file-path'>
|
||||
<Icon name='file audio'/>
|
||||
big_buck_bunny.mp3
|
||||
</Table.Cell>
|
||||
</Table.Row>
|
||||
</Table.Body>
|
||||
</Table>;
|
||||
|
||||
it('centres the glyph against the text rather than sitting it on the baseline', () => {
|
||||
cy.mountUI(fileRow, {theme: 'light'});
|
||||
|
||||
cy.get('td.file-path svg').should(($icon) => {
|
||||
const icon = $icon[0].getBoundingClientRect();
|
||||
// The text node's own box, which is what the eye compares the icon to.
|
||||
const range = document.createRange();
|
||||
range.selectNodeContents($icon[0].nextSibling);
|
||||
const text = range.getBoundingClientRect();
|
||||
|
||||
const iconCentre = icon.top + icon.height / 2;
|
||||
const textCentre = text.top + text.height / 2;
|
||||
expect(Math.abs(iconCentre - textCentre), 'icon centre vs text centre')
|
||||
.to.be.at.most(2);
|
||||
});
|
||||
});
|
||||
|
||||
it('leaves a gap between the glyph and the name', () => {
|
||||
// They touched: the folder icon ran into the first letter of its own folder name.
|
||||
cy.mountUI(fileRow, {theme: 'light'});
|
||||
|
||||
cy.get('td.file-path svg').should(($icon) => {
|
||||
const range = document.createRange();
|
||||
range.selectNodeContents($icon[0].nextSibling);
|
||||
const gap = range.getBoundingClientRect().left - $icon[0].getBoundingClientRect().right;
|
||||
expect(gap, 'gap between icon and filename').to.be.greaterThan(2);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('StatisticGroup separates its statistics with nothing but space', () => {
|
||||
/*
|
||||
* The group has no frame, no rules between cells and no surface of its own: the
|
||||
|
|
|
|||
|
|
@ -1183,6 +1183,51 @@ html[data-theme="night"] .wrolpi-status-failed {
|
|||
|
||||
/* ----------------------------------------------------------------- Tables */
|
||||
|
||||
/*
|
||||
* The header is a surface, not just bold text.
|
||||
*
|
||||
* Mantine gives `thead` no background at all, so the header showed whatever was
|
||||
* behind the table -- which is exactly what the unstriped body rows show. Bold
|
||||
* type was the only thing separating the two, and against a striped body the
|
||||
* header read as one more stripe. `--head` is the token for a recessed surface
|
||||
* and is distinct from both the panel and the stripe in all four themes.
|
||||
*/
|
||||
.mantine-Table-thead .mantine-Table-th {
|
||||
background: var(--head);
|
||||
}
|
||||
|
||||
/*
|
||||
* A full-strength rule under the header, because that is the one division in a
|
||||
* table that carries meaning: everything above it names, everything below it is
|
||||
* data. The rules between body rows are `--table-line` and much fainter -- see
|
||||
* the note there. Applied to the last header row so a grouped header does not
|
||||
* get a hard rule through the middle of itself.
|
||||
*/
|
||||
.mantine-Table-thead .mantine-Table-tr:last-child .mantine-Table-th {
|
||||
border-bottom: 2px solid var(--border);
|
||||
}
|
||||
|
||||
/*
|
||||
* Row rules are quieter than any other border in the interface. They only have
|
||||
* to let the eye track across a row; at `--border` strength, with a rule under
|
||||
* every row, a long table turns into a stack of boxes.
|
||||
*
|
||||
* `html` for specificity, not decoration: Mantine declares this variable on its
|
||||
* own hashed class, which ties with `.mantine-Table-table` and wins on source
|
||||
* order because its stylesheet is imported after this one.
|
||||
*/
|
||||
html .mantine-Table-table {
|
||||
--table-border-color: var(--table-line);
|
||||
}
|
||||
|
||||
/*
|
||||
* The outline around the whole table is not a row rule and keeps full strength;
|
||||
* Mantine draws both from the same variable, so it has to be restated.
|
||||
*/
|
||||
html .mantine-Table-table[data-with-table-border] {
|
||||
border-color: var(--border);
|
||||
}
|
||||
|
||||
/* A sortable header: a real button, so the column is reachable by keyboard. */
|
||||
.wrolpi-th-sort {
|
||||
display: inline-flex;
|
||||
|
|
|
|||
|
|
@ -737,6 +737,25 @@ describe('Table', () => {
|
|||
expect(screen.getByRole('cell', {name: 'example.com'})).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('draws no borders between columns', () => {
|
||||
/*
|
||||
* A full grid is a lot of ink to repeat what the column headings already say, and
|
||||
* it cost the header its identity: with no surface of its own, a bordered header
|
||||
* cell looked like one more cell in the grid. Mantine marks the choice on the
|
||||
* table, so the absence is assertable without a browser.
|
||||
*/
|
||||
const {container} = renderUI(<Table>
|
||||
<Table.Body>
|
||||
<Table.Row><Table.Cell>a</Table.Cell><Table.Cell>b</Table.Cell></Table.Row>
|
||||
</Table.Body>
|
||||
</Table>);
|
||||
|
||||
const table = container.querySelector('table');
|
||||
expect(table).not.toHaveAttribute('data-with-column-border');
|
||||
// The outline around the table is a different thing and stays.
|
||||
expect(table).toHaveAttribute('data-with-table-border');
|
||||
});
|
||||
|
||||
it('marks failed rows for the danger border', () => {
|
||||
const {container} = renderUI(<Table>
|
||||
<Table.Body>
|
||||
|
|
|
|||
|
|
@ -24,6 +24,8 @@ html[data-theme="light"] {
|
|||
--panel: #ffffff;
|
||||
--head: #e4e6e5;
|
||||
--border: #8f9698;
|
||||
/* The rule between table rows: softer than --border, which reads as a grid. */
|
||||
--table-line: #eceeed;
|
||||
--text: #22282a;
|
||||
--muted: #5f686a;
|
||||
|
||||
|
|
@ -66,6 +68,8 @@ html[data-theme="dark"] {
|
|||
--panel: #26292b;
|
||||
--head: #141617;
|
||||
--border: #4a5052;
|
||||
/* The rule between table rows: softer than --border, which reads as a grid. */
|
||||
--table-line: #343a3c;
|
||||
--text: #e6e8e6;
|
||||
--muted: #93999b;
|
||||
|
||||
|
|
@ -99,6 +103,8 @@ html[data-theme="night"] {
|
|||
--panel: #160404;
|
||||
--head: #000000;
|
||||
--border: #5c1010;
|
||||
/* The rule between table rows: softer than --border, which reads as a grid. */
|
||||
--table-line: #3a0d0d;
|
||||
--text: #e04a4a;
|
||||
--muted: #7a2525;
|
||||
|
||||
|
|
@ -128,6 +134,8 @@ html[data-theme="amber"] {
|
|||
--panel: #1a0f00;
|
||||
--head: #000000;
|
||||
--border: #5c3a08;
|
||||
/* The rule between table rows: softer than --border, which reads as a grid. */
|
||||
--table-line: #3d2708;
|
||||
--text: #ff9500;
|
||||
--muted: #8a5810;
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue