Repository navigation
Fix phone Back after a closed reader, and open desktop chapters with the reader - #507
Conversation
…selection On a phone, applyRoute calls setSheet before refresh(true). When Back returns to a result whose reader was closed, setSheet closes the reader and centres the selection while ui$active still holds the previous route's cards. geometry() then frames cards that are not on the new map, both sceneFrame() calls return null, and the explorer shows "Could not open this view" and drops the history snapshot. setSheet now calls calculateActive() before centerIfHidden(), so it frames the current projection's cards; geometry() is unchanged. The new WebKit test opens a result at 390 px, closes its reader, opens a result whose map shares no card with it (from search and by hash), and checks that Back keeps the reader closed, shows no error and keeps a history snapshot. It runs in the WebKit job. The fix takes the bundle to 144,000 bytes, so the explorer budget in the build script, publication and tests rises from 144,000 to 145,000 bytes. Jack approved this raise for this fix on 2026-10-29.
…rviews Since 8a4780f, updateProjection sets ui$scopeOverview for native shelves and boundary context cards as well as for authored-area overviews, so on desktop most chapters opened with only the 48 px reading handle, and zoom-out stopped at 0.9. A new ui$coarseOverview flag is true only for authored-area overviews and for scopes coarsened because they exceed 120 nodes. It now drives the desktop reader choice in applyRoute, the bp-dag-scope-overview class, clearSelection, syncSheetAccessibility, onStatusChange and the zoom-out floor in zoomAt. ui$scopeOverview still drives group navigation and phone map-first. The two assertions that 2c3c6b8 loosened in test_dag_viewer_browser.py are back in their e2a7f39 form: the reader is at least 340 px wide on load, and a map-card click leaves it open, without clicking the handle. The Node harnesses declare the new variable. A new WebKit test opens a hub chapter with three linked chapters and a 14-statement chapter with one unlinked statement at 1440x900, and checks the reader width, the chapter text, the missing overview class and zoom-out below 0.9. It runs in the WebKit job.
One Fixed entry under Unreleased covers the desktop reader default and the phone Back failure. The Explorer delivery entry now names the 145 KB budget that the phone fix raised.
CONTRIBUTING.md and docs/explorer-design.md still said the explorer bundle stays below 144 KB. The phone fix raised the enforced limit to 145,000 bytes and the bundle is now 144,080 bytes, so both lines now give the 145 KB budget that the changelog and the checks use.
The phone Back and reader-first WebKit tests were added only to the WebKit job in tests.yml, so make test-browser, which CONTRIBUTING.md says runs the browser tests as CI does, skipped both. The Makefile now lists the same nine files as the WebKit job.
|
Review at b422ab6: nothing blocks merge. Blockers: none. One finding was filed as a blocker and downgraded to a note after a second agent re-checked it (note 1). Separately, main has moved and GitHub now reports a conflict; the last paragraph says what the rebase needs. Notes. Line numbers are at b422ab6.
What was checked and holds. The agents ran the unmodified CI and merge state. At b422ab6, run 41002603872 is 9/9 green: eight jobs and the CLA check. WebKit has 207 passed and 1 skipped, against 203 and the same skip on main (run 40981081987); the four extra passes are the two new files. The guard runs show that the tests catch both bugs. On main's explorer code all six targeted tests fail (run 40995680060). With one fix removed, only that fix's tests fail (phone, reader). Main has since moved 48 commits to 8155f23, and GitHub reports #507 as conflicting, so the body's "Merges cleanly" line is out of date. A dry-run merge conflicts only in two files:
A scratch rebuild of the merged sources gives 144,080 bytes, under the limit. Rebase, run Posted by PR swarm: 320-329 Reviewer |
…d-reader-first # Conflicts: # CHANGELOG.md # autoform_cli/assets/blueprint-explorer.min.js
|
Head check at 898d31f: merge once CI passes. CI has passed. 898d31f merges main 8155f23 into b422ab6, the head reviewed above.
Posted by PR swarm: 320-329 Reviewer |
…d-reader-first # Conflicts: # CHANGELOG.md # autoform_cli/assets/blueprint-explorer.min.js
|
Head check at 13db62e: ready to merge. It merges main after #512, which conflicted in two places. CHANGELOG.md takes #512's "No size caps on published data" entry, which replaced #508's, and keeps this PR's entry. The explorer bundle is rebuilt from the merged sources at 143,570 bytes, and all five pins of the 145,000-byte limit survive. No source or test line of this PR changed. CI passes 9/9 (run 41166327902), and the head merges cleanly with current main. Posted by PR swarm: 320-329 Reviewer |
Follow-up to #442. It merged with two regressions against its last reviewed head, e2a7f39. A readiness check found them, and they were confirmed on main afterwards (#442 comment). Both are still on main 91490d7, after #444 (run 40986902670).
Problem
setSheetre-centres the selection beforerefresh(true)recomputes the active cards. Sogeometry()frames the previous route's cards, bothsceneFrame()calls return null, and the comparison throws.Change
setSheetcallscalculateActive()before it centres the selection on a closing phone reader.ui$coarseOverviewflag is true only for authored-area overviews and for scopes over 120 nodes.bp-dag-scope-overviewclass that collapses the reader.ui$scopeOverviewstill drives group navigation and phone map-first.scripts/build-explorer.mjs,dag_viewer.viewer_script, two intests/test_explorer_asset_bundle.py, andtests/test_graph_view_rendering.py.geometry(),fit(),sceneFrameand the layout are untouched.Tests
tests/test_integrated_phone_back_browser.py(WebKit, 390×844):dependencies/index.jsonwhose maps share no card;tests/test_integrated_reader_first_browser.py(WebKit, 1440×900): a hub chapter with three linked chapters, and a 14-statement chapter with one unlinked statement. Each must open with:tests/test_dag_viewer_browser.pythat 2c3c6b8 loosened.make test-browser.Notes
mapRequestedstill readsui$scopeOverview. At e2a7f39 focus went to the reading heading.ui$coarseOverview, or the 0.9 floor on authored-area overviews.tests/test_scoped_overview_browser.pycovers map-first for authored areas.Landing
check:explorerpasses. All five pins of the 145,000-byte limit survive the merge.tests/test_integrated_resize_observer.py.Review follow-ups not in this PR
The review at b422ab6 found nothing blocking. Its notes are left out to keep this fix small:
mapRequestedto read(sheetLayout() ? ui$scopeOverview : ui$coarseOverview)would focus the reading heading.&& overview.nodes.lengthto the flag would open its reader.Posted by PR swarm: 320-329 Reviewer