Repository navigation
Show a document without an outline in the full column in wide windows - #35
Merged
Merged
Conversation
A document with fewer than two H2 or H3 headings has no outline. In the wide layout (1180 px and wider) its text goes into the 220 px outline column. These checks fail on main at 1180 px.
The wide layout (1180 px and wider) always had three columns: 220 px for the outline, the text, and 220 px to balance. A document with fewer than two H2 or H3 headings has no outline, so its text was placed in the first column and shown 220 px wide. The reader now marks the pane when it has an outline, and the side columns apply only then.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A document with fewer than two H2 or H3 headings was shown in a 220 px column in any window 1180 px wide or wider. This gives the wide layout its outline columns only when the document has an outline.
What was reported
The owner opened a 2.1 KB status note in 0.7.0 (Dusk theme, text width Narrow, zoom 100%). The text sat in a column about 220 px wide starting near x=328, the H1 wrapped onto four lines, and there was no "On this page". The note has one H1, four short paragraphs (three open in bold) and one Mermaid flowchart, so no H2 or H3.
Stored settings, read from
HKCU\Software\LeanMark:Theme=dusk,ZoomPercent=100,TextWidth=narrow. The width setting is not involved: Wide and Full give the same 220 px column.Root cause
Triggering condition: the reader shows the wide layout (the width left for the document is 1180 px or more,
assets/reader.js:281) and the document has fewer than two H2/H3 headings, so it has no outline.assets/reader.css:1252-1254(v0.7.0)::root[data-layout="wide"] .content-grid { grid-template-columns: 220px var(--column) 220px; }applies to every document.assets/reader.js:615-618(v0.7.0): with fewer than two entries the outline getshidden, so it isdisplay: noneand takes no grid cell.<main>in the first track, the 220 px outline track, and the empty.layout-balance(assets/reader.css:1257) in the text track.So the text is 220 px wide whatever the width setting. In a 1,884 px wide page the column starts at x=323, which matches the report.
When it started: the rule is in the first commit (c332338, 0.1.0) as
@media (min-width: 1180px), and 0.6.0 moved it behinddata-layout="wide". Running the same check on the staged reader of v0.1.0, v0.5.0, v0.6.0 and v0.7.0 in headless Edge gives a 220 px column at 1300 and 1880 px in all four. It is not a 0.7.0 regression; it shows whenever a short note without subheadings is opened in a large window.Why the tests did not catch it: every column check in
tests/Invoke-ReaderBrowserSmoke.mjsuses a document with an outline (themes.md has four H2/H3). TheCOLUMNhelper even treats a hidden outline as no side columns, so that was the intended layout, but no check opened a document without an outline at 1180 px or wider.Reproduced three ways:
At 966 px the layout is the regular one and the column was already right, so the window in the report was wider than 966 px of page.
The fix
assets/reader.js: when the outline is built, the pane getsdata-outlineif the document has an outline and loses it if not (two lines).assets/reader.css: the three wide columns and the.layout-balancecell apply only to.content-grid[data-outline]. Without an outline the wide layout keeps the single column of the width setting.A selector with
:has()would do it in CSS alone, but the macOS build targets macOS 12.0, whose WebKit has no:has()before 12.3.Tests
Commit 1 adds the checks and fails on main; commit 2 is the fix.
Red on main (local, headless Edge):
FAIL with no outline, narrow at 1180 px is a centred 720 px column in one row {"width":220,...,"layout":"wide"}.Green with the fix: 215 checks pass locally. CI passes on Windows, Linux and macOS (run 42237828239).
Locally, the theme dot check
the theme button names System (Light)fails on this laptop on main too, because the test expects a light system and this laptop is set to dark. I ran the suite with--blink-settings=preferredColorScheme=1added to the browser arguments in a copy of the script. Not changed here.Mutation check
Each line was broken in a copy of the staged assets and the reader browser test run; all restored afterwards, suite green again.
[data-outline]from thegrid-template-columnsrulewith no outline, narrow at 1180 px is a centred 720 px column in one row(width 220).content-grid[data-outline]from the.layout-balanceruleremoveAttribute("data-outline")setAttribute("data-outline", "")Ctrl+Shift+L again gives Full, the window less its padding (1721 of 1217)Launch time and memory against 0.7.0
launch-timeworkflow on Windows, run 42245348765 (this branch against v0.7.0, 8 launches per document, taking turns). An earlier run, 42237841381, failed when GitHub lost contact with the runner and gave no numbers.Both time differences are under 1% and go in opposite directions. The files are 461 bytes larger (the new CSS and JS lines and comments). Linux and macOS were not timed.
Back-compat and rollback
Only the reader page changes. Documents with an outline get the same three columns as before. No setting or stored value changes. Rollback is reverting the fix commit. The merge runs the Build all platforms workflow (no release; that needs a tag).
Not changed here
tests/Invoke-DomSmoke.ps1) is not run in CI, so CI checks the reader page in Edge and Chrome, not inside WebView2. I checked the CI ZIP in the app by hand (table above).