diff --git a/app/src/App.css b/app/src/App.css index 8f9924d7..cbd5fb86 100644 --- a/app/src/App.css +++ b/app/src/App.css @@ -247,30 +247,6 @@ textarea.otp { color: #333; } -.inline-header h1 { - display: inline-block; -} - -.inline-header h2 { - display: inline-block; -} - -.inline-header h3 { - display: inline-block; -} - -.inline-header h4 { - display: inline-block; -} - -.inline-header h5 { - display: inline-block; -} - -.inline-header span { - margin-left: 1em; -} - .card-icon { min-height: 163px; text-align: center; diff --git a/app/src/components/Common.js b/app/src/components/Common.js index 6cbdec6e..0be2a362 100644 --- a/app/src/components/Common.js +++ b/app/src/components/Common.js @@ -810,14 +810,22 @@ export function InfoHeader({ required = false, ...props }) { - return
The level picks the size, so the type scale stays a scale: call sites choose a heading level for the document outline, never a font size. diff --git a/app/src/components/ui/Surfaces.tsx b/app/src/components/ui/Surfaces.tsx index 9321b2a0..6bad2f17 100644 --- a/app/src/components/ui/Surfaces.tsx +++ b/app/src/components/ui/Surfaces.tsx @@ -27,6 +27,14 @@ export interface HeaderProps extends Omit`. + */ + after?: React.ReactNode; } /** @@ -37,7 +45,7 @@ export interface HeaderProps extends Omit : )} {children} + {after && {after}} {subheader && {subheader}} diff --git a/app/src/components/ui/ui-layout.cy.js b/app/src/components/ui/ui-layout.cy.js index 0dd184ef..ac06e765 100644 --- a/app/src/components/ui/ui-layout.cy.js +++ b/app/src/components/ui/ui-layout.cy.js @@ -388,6 +388,94 @@ describe('a load reading ranks itself in the monochrome themes', () => { }); }); +describe('a heading keeps its trailing control on its own line', () => { + /* + * The defect this fixes: `.wrolpi-header` is a block, so a help icon placed after it + * wrapped to the next line -- every `HelpHeader` and `InfoHeader` in the app, most + * visibly "Root CA Certificate" on the Settings page. + * + * A pure layout claim, so jsdom cannot judge it: getBoundingClientRect returns zeros + * there, and containment alone (which the jest tests assert) does not prove two boxes + * share a line. + */ + themeNames.forEach((theme) => { + it(`sits the control beside the text in ${theme}`, () => { + cy.mountUI( + Help}> + Root CA Certificate + , + {theme}, + ); + + cy.get('.wrolpi-header-after').should(($after) => { + const control = $after[0].getBoundingClientRect(); + const text = $after[0].previousElementSibling.getBoundingClientRect(); + + // Beside, not below: they overlap vertically and the control is to the right. + expect(control.top, 'control starts above the text baseline') + .to.be.lessThan(text.bottom); + expect(text.top, 'text starts above the control baseline') + .to.be.lessThan(control.bottom); + expect(control.left, 'control is after the text').to.be.at.least(text.right); + }); + }); + }); + + it('does not stretch the heading to make room', () => { + // A wrapped control would make the heading two lines tall. One line is the claim. + cy.mountUI( + Help}> + Root CA Certificate + , + {theme: 'light'}, + ); + + cy.get('.wrolpi-header-text').should(($heading) => { + const control = Cypress.$('.wrolpi-header-after')[0].getBoundingClientRect(); + expect($heading[0].getBoundingClientRect().height, 'heading is one row tall') + .to.be.lessThan(control.height * 2); + }); + }); +}); + +describe('a heading opens a section rather than closing the last one', () => { + it('leaves room above a heading that follows something', () => { + /* + * With no top margin a heading sat flush against the panel above it and read as part + * of it -- "Tags" on /more/statistics touched the panel above with a 0px gap. + */ + cy.mountUI( + + Files + Tags + 34 tags + , + {theme: 'light'}, + ); + + cy.get('.wrolpi-header').should(($header) => { + const panel = Cypress.$('.wrolpi-panel')[0].getBoundingClientRect(); + const gap = $header[0].getBoundingClientRect().top - panel.bottom; + expect(gap, 'gap above the heading').to.be.at.least(12); + + // And more above than below, because the heading belongs to what follows it. + const below = Cypress.$('.wrolpi-panel')[1].getBoundingClientRect().top + - $header[0].getBoundingClientRect().bottom; + expect(gap, 'more air above the heading than below it').to.be.greaterThan(below); + }); + }); + + it('does not dent the top of its own container', () => { + // A blanket margin would push a heading that opens a Panel away from the Panel's edge. + cy.mountUI(Downloadsbody, {theme: 'light'}); + + cy.get('.wrolpi-header').should(($header) => { + expect(getComputedStyle($header[0]).marginTop, 'first child has no top margin') + .to.equal('0px'); + }); + }); +}); + describe('a table header is a surface, not just bold text', () => { themeNames.forEach((theme) => { it(`separates the header from every body row in ${theme}`, () => { diff --git a/app/src/components/ui/ui.css b/app/src/components/ui/ui.css index 2db28d2b..fd75994b 100644 --- a/app/src/components/ui/ui.css +++ b/app/src/components/ui/ui.css @@ -743,6 +743,30 @@ html[data-theme="night"] .wrolpi-tag::before { margin: 0 0 10px; } +/* + * A heading opens a section, so it needs more air above it than below — it belongs to + * what follows, not to what came before. With no top margin at all a heading sat flush + * against the panel above it, which read as part of that panel rather than as the start + * of the next thing (visible on /more/statistics, where "Tags" touched the panel above). + * + * `* +` rather than a blanket margin: a heading that opens a Panel or a page is already + * where it should be, and pushing it down would just dent the top of its own container. + */ +* + .wrolpi-header { + margin-top: 22px; +} + +/* + * Trailing content keeps the heading's baseline but not its size: a help button rendered + * at heading scale is a 26px glyph next to a 17px word. + */ +.wrolpi-header-after { + display: inline-flex; + align-items: center; + font-size: 13px; + font-weight: 400; +} + .wrolpi-header-dividing { border-bottom: 1px solid var(--border); padding-bottom: 6px; diff --git a/app/src/components/ui/ui.test.js b/app/src/components/ui/ui.test.js index eb7d2663..328a6c52 100644 --- a/app/src/components/ui/ui.test.js +++ b/app/src/components/ui/ui.test.js @@ -346,6 +346,26 @@ describe('Header', () => { expect(container.querySelector('svg')).toBeInTheDocument(); expect(screen.getByText('Paste this into the extension')).toBeInTheDocument(); }); + + it('puts trailing content inside the heading, not after its wrapper', () => { + /* + * The whole point of the slot. `.wrolpi-header` is a block, so anything placed after + * it starts a new line -- which is what happened to every help icon in the app when + * these headings stopped being a bare ``. Inside the heading element it joins + * the existing flex row. + */ + const {container} = renderUI( + Help}>Root CA Certificate); + + const heading = container.querySelector('h3'); + expect(heading).toContainElement(screen.getByRole('button', {name: 'Help'})); + }); + + it('renders nothing extra when there is no trailing content', () => { + const {container} = renderUI(Root CA Certificate); + + expect(container.querySelector('.wrolpi-header-after')).toBeNull(); + }); }); describe('PathInput', () => {
body