Show the thread panel over the page instead of modally over it - #144
Conversation
c73fbd6 to
8afe5f0
Compare
leaf-agent
left a comment
There was a problem hiding this comment.
The shape is right and it deletes more than it adds. I ran the five tests the description claims outright: all green, and test_covering_panel_keeps_toasts_on_screen_and_clear_of_the_footer fails byte-identically on clean main (toast left=3, panel left=-15 right=305), so that row's attribution to #121 holds.
One consequence the description misses, and it collides with an invariant the suite already states.
Tab now walks out of a covering sheet onto page content the sheet paints over. Leaving the top layer doesn't only drop the ::backdrop — it drops the inertness that was keeping the covered page out of the sequential focus order. Measured on ADDRESSED_PAGE at 320×600 with Threads open, tabbing on from the panel's Send button: before, focus cycles inside the panel forever; after, it continues onto #lk1, #lk2, #dsc-head and an lf-pick, every one of them under the opaque sheet with no ring the reader can see.
That is the thing test_go_page_is_inert_while_the_panel_covers_the_page is written to prevent — its docstring says "focus cannot honestly return to that page while keeping the panel open", and g p is refused for exactly that reason. Tab now grants what the address chord is forbidden to. The runtime already treats this class as a defect for its own page affordances: .lf-mark-note:is(:focus-visible, .lf-focus-visible) and .lf-visual-action:focus-visible both re-place themselves position: fixed; z-index: 9050 at the top-left on focus, so a focused Leaf control can never be the thing hidden behind the panel. Ordinary authored markup gets nothing.
Blanket inert on the page isn't the answer — it would take the text selection test_selection_hints_do_not_name_page_content_behind_a_covering_panel depends on. But the honest reading of the strip is "live where it is painted", and the tab order is the half of "live" with no visual answer at 320px, where nothing of the page is painted at all. Worth settling before this lands rather than after.
Two smaller notes, both in the change's favour:
- The scrim isn't the only paint that moves.
syncLayoutalready lifts the toast abovepanelFootand gives the key line the window's full width over a covering sheet ("A covering panel is handled by the lift above and leaves the line the window's full width"), and modality was making both inert — a toast raised over a covering panel was painted behind it. Same fixture, 700×900, panel/toast overlap centre:(255,254,251)before (the panel's card),(210,156,87)after. Those are the design working for the first time, not regressions, but they belong in the description alongside the scrim. showPanelLayerreadsdocument.activeElementandpanel.contains(invoker)where the module already hasfocused()andcontainsAcross()injected for this question, one line above ininPanel.keyboard/scopes.jssays why they exist — "Focus inside a shadow tree retargets to the host, while the label transition above can report body or a containing element until its click completes". I couldn't make it bite on any path I traced, so it is consistency rather than a bug.
Separately: the branch is CONFLICTING against main since #132 landed, so it needs a rebase before it can go anywhere.
Provenance and how the tab walk was measured
The measurements were taken on the pre-rebase head c73fbd6 against base c6ced94, before the force-push to 8afe5f0. I diffed the two versions of the patch: they are identical except for three comment lines dropped from setPanel (Focus is asked for in the same call because its target is threadsBox…), which #138 had introduced. Every line the finding rests on — panel.show() in place of showModal(), and the deleted .lf-panel::backdrop rule — is verbatim the same at 8afe5f0.
Probe run on the merged tree and on a base worktree, same fixture and viewport, 14 Tab presses, resolving through shadow roots and testing each landing against the panel's box:
page, _ = open_page(browser, serve(ADDRESSED_PAGE))
resized(page, 320, 600)
page.locator(".lf-threads-toggle").click()
# ... 14x page.keyboard.press("Tab"), recording activeElement and whether it
# overlaps .lf-panel while sitting outside itBase (:modal true) — every landing inPanel: True, cycling lf-find-box → react trigger → textarea → Send → panel close button.
Branch (:modal false) — after Send: lf-key-more, BODY, A#lk1 [24, 272], A#lk2 [149, 353], SUMMARY#dsc-head [39, 403], SPAN.lf-pick [41, 346], against a panel spanning left -15 … right 305.
The panel became a <dialog> in 335bf9a and is raised with showModal() while it covers the page. A modal dialog makes the rest of the document inert, so the banner the panel is closed from, the workspace toggle beside it, and the strip of page still showing all stopped answering. Both spellings also run the dialog focusing steps, so opening the panel took focus off the control that opened it. Show the panel at either posture and give the invoker its focus back. The scroll lock a covering sheet needs is already the stylesheet's, and Escape is already the ladder's, so nothing modality was carrying is lost. Closes #139
8afe5f0 to
2b462cc
Compare
|
The one row your table leaves open — It is not #121's Measured
15px is the UA's |
Problem
335bf9a made
.lf-panela<dialog>andsyncPanelLayerraised it withshowModal()whenever it covers the page. A modal dialog makes the rest of the document inert, and covering is the posture in which the page most needs to stay live — so the banner toggle that opened the panel stopped closing it, the decisions toggle beside it stopped switching workspace, and the strip of page still showing beside the sheet stopped answering a selection hint. Separately, bothshow()andshowModal()run the browser's dialog focusing steps, so opening the panel took focus off the control that opened it.Those are the six unowned tests in #139. Five are the inertness; the sixth is the focus.
Solution
The panel is shown, never shown modally, at either posture, and the invoker gets its focus back. What modality was carrying is already owned elsewhere and stays: the covering sheet's root scroll lock is the stylesheet's
overflow-y: hiddenunderCOVERING, and Escape is the ladder's —browserDismissesTopLayerhands Escape to the platform only for layers the platform really owns, which the keyboard reference and the page map sheet still are.skills/leaf/CLAUDE.mdalready states the same reading from the other direction:presentPagedemotes authoredshowModal()calls to non-modal dialogs precisely so a modal's top-layer inertness cannot disable the recovery chrome.That deletes rather than adds machinery:
panelModal,ignoreNextClose, the close-and-reopen across the covering breakpoint, theresizeandchangere-syncs that existed only to switch modality, thepanelFocusTargetwiring, and thecloselistener that only a native modal dismissal could reach.syncPanelLayerbecomesshowPanelLayer, called once fromsetPanel— still beforerenderPanel, for the reason #138 gave it.The one user-visible consequence is the scrim: a non-modal dialog paints no
::backdrop, so the.lf-panel::backdroprule goes with it. That reads as the point rather than a cost — a scrim over a page the reader can now press would be saying the opposite of what the page does.Which of the six this closes
Measured on this branch, rebased on
mainat 0b7f148 (so #121, #138 and #141 are in the base).test_selection_hints_do_not_name_page_content_behind_a_covering_paneltest_workspaces_replace_each_other_instead_of_stacking[500]test_coarse_pointer_chrome_gives_its_compact_controls_humane_aimstest_esc_hands_the_page_back_after_it_has_closed_the_last_panelmainby #141, kept green heretest_the_responsive_action_shelf_keeps_primary_actions_in_reachtest_coarse_pointer_resize_reach_stays_reachable_without_trapping_scrollmain's — see belowWhat this does not fix
test_coarse_pointer_resize_reach_stays_reachable_without_trapping_scrollneeded both this and #121, and with both it is still red — on a different, earlier failure that #121 brought and this change does not touch. Walking it forward, each row a local-n0run:mainat 1ca4f40 (pre-#121).lf-decisionsclick —<dialog … class="lf-ui lf-panel open"> intercepts pointer eventsassert edge["left"] >= -0.1→-12wait_for_functionon.lf-threadsscrollTopafter a touch swipemainpost-#121mainat 0b7f148 + thisLine 1006 is earlier than the failures the modal caused, so it was masked until #121 landed; it is green on 1ca4f40 and on 1ca4f40 + this. It belongs to #121's
overflow-y: scroll, is live onmainnow, and needs its own fix.Relation to #141
#141 landed while this was being measured. It fixed the focus half by extending
syncPanelLayer's restore arm while keepingshowModal(), sotest_esc_hands_the_page_back_after_it_has_closed_the_last_panelis already green onmain; this rebases onto it and replaces that arm rather than competing with it. With modality gone the restore has one case instead of four, andfocusEnteredLayer,panelFocusTargetandreturningall go with it.Testing
Runs
On
mainat 0b7f148:The one failure is the row above. Earlier, on the pre-rebase base (1ca4f40), the owning files ran
5 failed, 77 passedfortest_render_controls.py+test_render_semantic_selection.pyand2 failed, 45 passedfortest_render_conversations.py; every one of those seven reproduced identically on cleanmain— five of the reserved-gutter shape #121 has since taken, plustest_a_settlement_in_a_reply_leaves_its_own_anchor_on_the_pageandtest_a_mark_in_the_layer_promises_no_press_the_layer_will_not_take, which are unrelated and untouched here.No new test: the six already own the behaviour, and each names it in its own docstring.
The visual delta, measured rather than described
Same fixture, same viewport, same state —
LONG_PAGEat 700×900 with the Threads panel open, which is the covering posture. Panel geometry is identical across the two (x=265 y=96 w=420 h=804);dialog:modalreadstruebefore andfalseafter.Every pixel outside the panel changes, and only by the scrim coming off:
(222,221,219)is paper undercolor-mix(in srgb, var(--ink) 12%, transparent);(250,249,246)is paper. The banner was inside the dimmed area too, which is the visible half of the bug: the chrome the reader closes the panel from was drawn as if it were unavailable, and it was.Closes #139