diff --git a/app/src/App.css b/app/src/App.css index c05d1ec5..eb5ae988 100644 --- a/app/src/App.css +++ b/app/src/App.css @@ -336,12 +336,94 @@ textarea.otp { cursor: pointer; } +/* + * A table that truncates to the width it has, rather than growing past it. + * + * `table-layout: fixed` is what makes the truncation possible at all. Under the default + * `auto` layout a table is as wide as its columns want to be and the container simply + * overflows -- on a 540px phone the downloads tables came out 676px and 727px, so Completed + * At and Control sat off the right edge and the buttons could not be reached without + * scrolling sideways. Fixed layout inverts that: the columns get the table's width divided + * between them, and the one marked `column-ellipsis` gives up what it cannot show. + * + * The old rule tried to do this with `max-width: 0` and `min-width: 25em`. The max-width was + * the usual trick for forcing an ellipsis under `auto` layout; the min-width was there so the + * text column stayed readable -- but 25em is 385px, and a floor wider than the screen is what + * made the overflow certain. Under fixed layout neither is needed: the column width comes + * from the header, and every other column is sized to its content so the ellipsis column + * keeps the remainder. + * + * Columns that must fit something exact -- a checkbox, a group of buttons -- state a width on + * their header cell. Any column that does not is sized by what is left over. + */ +/* + * `table.` rather than the class alone: Mantine sets `table-layout` on its own + * `.mantine-Table-table`, which is the same specificity and comes later in the cascade, so + * the bare class silently lost and the table stayed `auto` -- with every other property here + * applying, which made it look like the rule had worked. + */ +table.table-ellipsis { + width: 100%; + table-layout: fixed; +} + .table-ellipsis td.column-ellipsis { white-space: nowrap; overflow: hidden; text-overflow: ellipsis; - max-width: 0; /* Ensures the content doesn't exceed the cell's width */ - min-width: 25em; /* About iPhone SE width */ +} + +/* + * The one column width that has to change with the screen. + * + * Most headers here are sized to hold their own label on one line at any width -- a wrapped + * label is a fallback, not a layout. "Download Frequency" is the exception: 11.7em on one + * line, which a phone cannot spare when four other columns need room, and it is one of five + * columns rather than one of four. So it wraps below the tablet breakpoint and gets its line + * back above it, where the width costs nothing. + * + * 700px is the app's `tablet` breakpoint (see `AppMedia` in contexts.js). + */ +.table-ellipsis th.col-frequency { + width: 7em; +} + +@media (min-width: 700px) { + .table-ellipsis th.col-frequency { + width: 12em; + } +} + +/* + * A header wraps instead of running into the next column. + * + * `.wrolpi-th-sort` is `white-space: nowrap` so a sortable label never splits from its arrow. + * That is right under `auto` layout, where the column widens to fit; under fixed layout the + * column does not widen, so "Completed At" overran its cell and printed on top of "Control". + * Wrapping to two lines is the only thing a fixed column can do with a label too long for it. + * + * `min-width: 0` as well as the wrap: the button is a flex container, and its text is an + * anonymous flex item whose automatic minimum is its longest word. Without this the label + * still refuses to go below "Completed" and still overruns. A column narrower than its + * longest word is a width to fix at the call site, not something CSS can hide. + */ +.table-ellipsis th .wrolpi-th-sort { + display: flex; + white-space: normal; + min-width: 0; + text-align: left; +} + +/* + * Grouped buttons keep their size in a fixed column. + * + * `ButtonGroup` is a nowrap flex row whose children may shrink, and in a column too narrow + * for them they did: the Stop button on a pending download was squeezed to 23px against its + * neighbour's 40px, a target too small to hit and visibly clipped. The column is sized for + * the widest group it has to hold (three buttons), so the buttons themselves should not give. + */ +.table-ellipsis td .mantine-ButtonGroup-group > * { + flex-shrink: 0; } .card-title-ellipsis { diff --git a/app/src/components/SortableTable.js b/app/src/components/SortableTable.js index 6dd82f9c..5c10fdb0 100644 --- a/app/src/components/SortableTable.js +++ b/app/src/components/SortableTable.js @@ -44,18 +44,24 @@ export class SortableTable extends React.Component { const data = this.props['data'] ? this.sortData(this.props['data']) : null; const tableHeader = (spec) => { - const {key, text, sortBy} = spec; + // `width` is how a column that must fit something exact -- a checkbox, a group of + // buttons -- states its size. Under `table-layout: fixed` (see `.table-ellipsis`) + // the header cell is where a column's width is declared; columns that name none + // divide what is left, which is what the truncating column wants. + const {key, text, sortBy, width} = spec; + const style = width ? {width} : undefined; if (sortBy) { const active = sortColumn === key; return this.changeSort(key)} > {text} ; } else { - return {text} + return {text} } } diff --git a/app/src/components/admin/Downloads.js b/app/src/components/admin/Downloads.js index 66438956..0b267b68 100644 --- a/app/src/components/admin/Downloads.js +++ b/app/src/components/admin/Downloads.js @@ -584,6 +584,16 @@ export function OnceDownloadsTable({downloads, fetchDownloads}) { } }; + /* + * Widths for everything except the URL, which takes what is left and truncates. See + * `.table-ellipsis` in App.css: under fixed layout a column with no width stated gets an + * equal share, which would give a checkbox as much room as a URL. + * + * The numbers are what the content needs, measured: a checkbox is 2.5em; "Completed At" is + * 9.5em on one line with its sort arrow, which is narrow enough to keep on a phone rather + * than wrapping it there; and Control is 9.5em, which holds the widest button group a row + * can have (a failed download shows delete, edit and retry). + */ const tableHeaders = [ { key: 'select', @@ -592,11 +602,13 @@ export function OnceDownloadsTable({downloads, fetchDownloads}) { indeterminate={selectedIds.length > 0 && selectedIds.length < (downloads?.length || 0)} onChange={toggleSelectAll} />, - sortBy: null + sortBy: null, + width: '2.5em', }, {key: 'url', text: 'URL', sortBy: i => i.url.toLowerCase()}, - {key: 'completed_at', text: 'Completed At', sortBy: i => i.last_successful_download || ''}, - {key: 'control', text: 'Control', sortBy: null}, + {key: 'completed_at', text: 'Completed At', sortBy: i => i.last_successful_download || '', + width: '9.5em'}, + {key: 'control', text: 'Control', sortBy: null, width: '9.5em'}, ]; const rowFunc = (download) => ( @@ -654,12 +666,22 @@ export function RecurringDownloadsTable({downloads, fetchDownloads, onDelete}) { if (downloads && downloads.length >= 1) { return + {/* Widths in em, not percentages: what each of these columns has to hold is a + fixed amount of text or a fixed number of buttons, and a percentage of a + phone's width is not that. At 50% the URL column was 385px on a 540px + screen and pushed Next and Control off the edge. Only the URL is elastic + now -- it takes the remainder and truncates, which is its job. + + "Download Frequency" is the one label too long to keep on one line on a + phone, so its width comes from `.col-frequency` in App.css, which widens at + the tablet breakpoint. The rest are sized to hold their label unwrapped at + any width. */} - URL - Download Frequency - Completed At - Next - Control + URL + Download Frequency + Completed At + Next + Control diff --git a/app/src/components/admin/Status.js b/app/src/components/admin/Status.js index a1f99785..9cbf23c2 100644 --- a/app/src/components/admin/Status.js +++ b/app/src/components/admin/Status.js @@ -348,11 +348,15 @@ export function StatusPage() { Top Processes
+ {/* The three readings are short and fixed; Command takes the rest and + truncates. This table already asked for `.table-ellipsis` and marked + its Command cell, so it overflowed a phone the same way the downloads + tables did -- a long command line has no natural width. */} Command - CPU % - Mem % - PID + CPU % + Mem % + PID diff --git a/app/src/components/ui/ui-layout.cy.js b/app/src/components/ui/ui-layout.cy.js index bd337d21..ef652887 100644 --- a/app/src/components/ui/ui-layout.cy.js +++ b/app/src/components/ui/ui-layout.cy.js @@ -1,6 +1,6 @@ import React from 'react'; import { - ActionInput, Button, Card, Header, Icon, IconButton, IconStack, Loading, MultiSelect, Panel, PathInput, + ActionInput, Button, ButtonGroup, Card, Header, Icon, IconButton, IconStack, Loading, MultiSelect, Panel, PathInput, Group, Message, Pagination, Placeholder, Progress, SearchBox, Statistic, StatisticGroup, Status, TabBar, Table, tabClassName, TextInput, } from './index'; @@ -1448,6 +1448,159 @@ describe('table rules', () => { }); }); +describe('a truncating table fits the width it is given', () => { + /* + * The downloads tables ran off the side of a phone: 676px and 727px of table in a 540px + * window, so Completed At and Control were off the right edge and their buttons could not + * be reached without scrolling sideways. `.table-ellipsis` was trying to truncate under + * `table-layout: auto`, where a table is as wide as its columns want to be, and the + * `min-width: 25em` floor on the truncating column made overflowing certain. + * + * None of this is visible in jsdom -- it is all resolved widths, and the fix turns on + * whether our `table-layout` beats the one Mantine sets on its own class. It did not, at + * first: every other property in the rule applied, so the table looked fixed and stayed + * auto. These measure the rendered table instead. + */ + const wideTable =
+ + + + + + + + + + + + + + + + +
URLCompleted AtControl
+ https://example.com/a/very/long/path/that/no/phone/could/ever/show/in/full + 3 days + + + + + +
; + + const mountNarrow = () => cy.viewport(400, 700).then(() => cy.mountUI(wideTable)); + + it('does not grow past its container on a phone', () => { + mountNarrow(); + + cy.get('table.table-ellipsis').should(($t) => { + const table = $t[0]; + // The rule that does the work. Asserted directly because it is the one that lost + // to Mantine's own class, silently, while the rest of the rule applied. + expect(getComputedStyle(table).tableLayout, 'fixed layout').to.equal('fixed'); + expect(table.getBoundingClientRect().width, 'table fits its container') + .to.be.at.most(table.parentElement.clientWidth + 1); + }); + }); + + it('spends the leftover width on the column that truncates', () => { + mountNarrow(); + + cy.get('td.column-ellipsis').should(($cell) => { + const cell = $cell[0]; + // It gives up what it cannot show... + expect(cell.scrollWidth, 'the long URL is truncated').to.be.greaterThan(cell.clientWidth); + // ...and it is still the widest column, which is the point of truncating it rather + // than any of the others. A 25em floor made it the widest by overflowing instead. + const others = [...cell.closest('tr').children].filter(td => td !== cell); + others.forEach(td => { + expect(cell.getBoundingClientRect().width, 'the URL keeps the remainder') + .to.be.greaterThan(td.getBoundingClientRect().width); + }); + }); + }); + + it('wraps a header rather than printing it over the next column', () => { + // The fixture gives "Completed At" 8em deliberately -- less than the 9.33em it needs + // on one line -- because wrapping is the behavior under test. `.wrolpi-th-sort` is + // nowrap, so before this the label ran on into Control and the two printed on top of + // each other. The real tables are sized to hold their labels; this is the fallback. + mountNarrow(); + + cy.get('table.table-ellipsis thead th').should(($ths) => { + [...$ths].forEach(th => { + const inner = th.firstElementChild; + if (!inner) return; + expect(inner.getBoundingClientRect().right, + `"${th.textContent.trim()}" stays inside its column`) + .to.be.at.most(th.getBoundingClientRect().right + 1); + }); + }); + }); + + it('gives a long header its own line back on a desktop', () => { + /* + * A wrapped label is the fallback, not the layout. "Download Frequency" is 11.7em on + * one line, which a phone cannot spare beside four other columns, so `.col-frequency` + * carries a narrow width below the tablet breakpoint and a wide one above it. Without + * the media query the label wrapped at every width, including a 1400px desktop with + * hundreds of pixels going spare. + */ + const frequencyHeader = + + + + + + + +
URLDownload Frequency
https://example.com/x30 Days
; + + const widthOfFrequency = () => cy.get('th.col-frequency') + .then(($th) => $th[0].getBoundingClientRect().width); + + cy.viewport(400, 700); + cy.mountUI(frequencyHeader); + widthOfFrequency().then((narrow) => { + cy.viewport(1200, 700); + cy.mountUI(frequencyHeader); + widthOfFrequency().then((wide) => { + expect(wide, 'the column widens past the tablet breakpoint') + .to.be.greaterThan(narrow); + // Wide enough for the whole label on one line, which is the point of widening. + cy.get('th.col-frequency').should(($th) => { + const th = $th[0]; + const cs = getComputedStyle(th); + const contentW = th.getBoundingClientRect().width + - parseFloat(cs.paddingLeft) - parseFloat(cs.paddingRight); + const probe = document.createElement('span'); + probe.style.cssText = 'position:absolute;visibility:hidden;white-space:nowrap'; + probe.textContent = th.textContent.trim(); + th.appendChild(probe); + const need = probe.getBoundingClientRect().width; + probe.remove(); + expect(need, '"Download Frequency" fits on one line').to.be.at.most(contentW); + }); + }); + }); + }); + + it('keeps grouped buttons at full size in a narrow column', () => { + // ButtonGroup is a nowrap flex row whose children shrink: in a column too narrow the + // Stop button came out 23px against its neighbour's 40px -- clipped, and too small to + // hit. The column is sized for three buttons, so none of them should give. + mountNarrow(); + + cy.get('td .mantine-ButtonGroup-group button').should(($btns) => { + expect($btns.length, 'three buttons, the widest group a row can hold').to.equal(3); + const widths = [...$btns].map(b => b.getBoundingClientRect().width); + widths.forEach(w => expect(w, 'button is not squeezed').to.be.at.least(30)); + expect(Math.max(...widths) - Math.min(...widths), 'all three are the same width') + .to.be.at.most(1); + }); + }); +}); + 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