mirror of
https://github.com/lrnselfreliance/wrolpi.git
synced 2026-10-26 19:01:39 -04:00
test: act on review — two of the new table guards could not fail
The jest column-border test asserted the TABLE lacks `data-with-column-border`.
Mantine puts that marker on the cells, never on the table, so the expectation
held whether column borders were on or off. It now looks for the marker
anywhere in the tree.
I did plant a regression for this one -- and ran it against cypress only, where
a different test caught it. The plant was right and the suite was wrong, which
is a new way to get a false green and worth naming: run every suite that claims
the behaviour, not the one that happens to be open.
The header-surface test fell back to `getComputedStyle(documentElement)
.backgroundColor` for an unstriped row. `body` carries the page colour and the
root element carries none, so that is `rgba(0, 0, 0, 0)` and any opaque header
passed. It now compares against the `--bg` token, which is what actually shows
through an unstriped row, and additionally asserts the body really is striped
so both kinds are covered. Planted the exact case the old form could not see
-- a header painted `--bg` -- and it goes red in all four themes.
The file-row fixture rendered a 16px icon; the app renders 24. A file row
calls `<FileIcon size={null}/>`, and a null size falls past Icon's 'small'
default to Tabler's 24 -- the harder centring case, and the one the CSS comment
describes. Added the two-glyph row as well, since a folder that is also ignored
draws two icons before its name and those touched each other too.
The header-rule-width assertion now runs per theme with the rest of the suite.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Roland
parent
87c1e53be4
commit
12d7ef377f
2 changed files with 74 additions and 23 deletions
|
|
@ -192,33 +192,43 @@ describe('a table header is a surface, not just bold text', () => {
|
|||
expect(header, 'the header paints a surface of its own')
|
||||
.to.not.match(/^rgba\(.*,\s*0(\.0+)?\)$/);
|
||||
|
||||
const page = hexToRgb(getComputedStyle(document.documentElement)
|
||||
.getPropertyValue('--bg').trim());
|
||||
const rows = [...Cypress.$('tbody tr')].map(row => {
|
||||
// An unstriped row paints nothing, so what shows is the page behind it.
|
||||
/*
|
||||
* An unstriped row paints nothing, so what shows through is the page.
|
||||
* That is `--bg`, taken from the token -- NOT
|
||||
* getComputedStyle(documentElement).backgroundColor, which is
|
||||
* `rgba(0, 0, 0, 0)` because `body` carries the page colour and the root
|
||||
* element carries none. Comparing an opaque header against that string
|
||||
* can never fail, which is how this passed before it was fixed.
|
||||
*/
|
||||
const own = getComputedStyle(row).backgroundColor;
|
||||
return /^rgba\(.*,\s*0(\.0+)?\)$/.test(own)
|
||||
? getComputedStyle(document.documentElement).backgroundColor
|
||||
: own;
|
||||
return /^rgba\(.*,\s*0(\.0+)?\)$/.test(own) ? page : own;
|
||||
});
|
||||
expect(rows.length).to.be.greaterThan(2);
|
||||
// Both kinds have to be present or the comparison only covers one of them.
|
||||
expect(new Set(rows).size, 'the body is striped').to.equal(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'});
|
||||
it(`rules off the header more heavily than it rules between rows in ${theme}`, () => {
|
||||
/*
|
||||
* 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. Per theme, because a theme is free to restyle borders.
|
||||
*/
|
||||
cy.mountUI(sampleTable, {theme});
|
||||
|
||||
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));
|
||||
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));
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -276,17 +286,36 @@ describe('a file row lines its icon up with its name', () => {
|
|||
* Purely a question of layout: jsdom returns zeros from getBoundingClientRect, so
|
||||
* nothing there can tell a centred glyph from one hanging off a baseline.
|
||||
*/
|
||||
/*
|
||||
* 24px, which is what the app renders. A file row calls `<FileIcon size={null}/>`, and
|
||||
* a null size falls past Icon's 'small' default to Tabler's own 24. The default 16px
|
||||
* icon is a much easier centring case and can look fine while the real one is visibly
|
||||
* off, so a fixture using it would be testing a size the file browser never draws.
|
||||
*/
|
||||
const fileRow = <Table>
|
||||
<Table.Body>
|
||||
<Table.Row>
|
||||
<Table.Cell className='file-path'>
|
||||
<Icon name='file audio'/>
|
||||
<Icon name='file audio' size={24}/>
|
||||
big_buck_bunny.mp3
|
||||
</Table.Cell>
|
||||
</Table.Row>
|
||||
</Table.Body>
|
||||
</Table>;
|
||||
|
||||
// A folder that is also ignored draws two glyphs before its name; they touched too.
|
||||
const folderRow = <Table>
|
||||
<Table.Body>
|
||||
<Table.Row>
|
||||
<Table.Cell className='file-path'>
|
||||
<Icon name='folder' size={24}/>
|
||||
<Icon name='eye slash' size={24}/>
|
||||
config/
|
||||
</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'});
|
||||
|
||||
|
|
@ -315,6 +344,27 @@ describe('a file row lines its icon up with its name', () => {
|
|||
expect(gap, 'gap between icon and filename').to.be.greaterThan(2);
|
||||
});
|
||||
});
|
||||
|
||||
it('keeps two glyphs apart, and both clear of the name', () => {
|
||||
cy.mountUI(folderRow, {theme: 'light'});
|
||||
|
||||
cy.get('td.file-path svg').should(($icons) => {
|
||||
expect($icons.length, 'two glyphs').to.equal(2);
|
||||
const [folder, ignored] = [...$icons].map(icon => icon.getBoundingClientRect());
|
||||
expect(ignored.left - folder.right, 'gap between the two glyphs')
|
||||
.to.be.greaterThan(2);
|
||||
|
||||
const range = document.createRange();
|
||||
range.selectNodeContents($icons[1].nextSibling);
|
||||
expect(range.getBoundingClientRect().left - ignored.right, 'gap before the name')
|
||||
.to.be.greaterThan(2);
|
||||
|
||||
// And the second glyph is centred on the name as well, not just the first.
|
||||
const text = range.getBoundingClientRect();
|
||||
expect(Math.abs((ignored.top + ignored.height / 2) - (text.top + text.height / 2)),
|
||||
'second glyph centre vs text centre').to.be.at.most(2);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('StatisticGroup separates its statistics with nothing but space', () => {
|
||||
|
|
|
|||
|
|
@ -741,8 +741,10 @@ describe('Table', () => {
|
|||
/*
|
||||
* 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.
|
||||
* cell looked like one more cell in the grid.
|
||||
*
|
||||
* The marker goes on the CELLS. Asserting the table lacks it is free -- the table
|
||||
* never carries it either way -- which is what this test did at first.
|
||||
*/
|
||||
const {container} = renderUI(<Table>
|
||||
<Table.Body>
|
||||
|
|
@ -750,10 +752,9 @@ describe('Table', () => {
|
|||
</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');
|
||||
expect(container.querySelector('[data-with-column-border]')).toBeNull();
|
||||
// The outline around the table is a different thing, and it does live on the table.
|
||||
expect(container.querySelector('table')).toHaveAttribute('data-with-table-border');
|
||||
});
|
||||
|
||||
it('marks failed rows for the danger border', () => {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue