web: fix dropdown dismiss, retained-store order, modal hit-testing - #65
Merged
Merged
Conversation
- insert_at appended the inserted value a second time for every non-tail insert, leaving stale child ids that crashed apply-batch with "unknown node" after navigation - every mounted dropdown's document pointerdown emitted Dismiss on outside presses, so clicks inside sibling menus closed open pickers and removed the press target before click could commit; gate Dismiss on the menu actually being open and treat the whole dropdown group (nested submenu positioners are portal siblings, not descendants) as inside - restore pointer-events on .lui-modal-layer so dialogs are clickable inside the pointer-events:none portal - give --destructive-foreground a light-on-destructive value in the light theme - restore the flutter example app icon referenced by web asset loaders
Contributor
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
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.
Summary
Fixes for bugs found exercising the web gallery demo after #63.
insert_atcorrupted the retained store's child order (lui_web_store.ml): the[]base case didList.rev_append acc [value], re-appending the inserted element after the accumulator tail for every non-tail insert. Subsequentreplace_children/patches then referenced stale node ids and crashed apply-batch withunknown node(seen navigating back to the Toast page). Now returnsList.rev acc, matching the originalweb.cljc.lui_web_menu.ml): every mountedDropdownMenuinstalls a documentpointerdownthat emittedDismisson outside presses — including presses inside other pickers' menus on the same page, which closed the open picker and detached the click target beforeclickfired. The handler now only emitsDismisswhile that menu is actuallydata-open, anddropdown_group_contains_eventtreats the whole dropdown group as inside (nested submenus get their own positioner in the portal, so they're DOM siblings, not descendants).lui.css):.lui-modal-layerlives inside.lui-popup-portal(pointer-events: none) but never re-enabled hit-testing. Addedpointer-events: auto. This was a latent upstream bug — the original CSS had the same hole.--destructive-foregroundinvisible in light theme: it aliased--destructiveitself; nowoklch(0.985 0 0).examples/components/flutter/.../app_icon_128.png(referenced by the web demo's asset loader; deleted during the OCaml migration).Verified: overlay 38/38, simulator 15/15, unit 44/44, hot-reload 1/1, firefox 1/1 — plus a Playwright repro covering select/submenu mouse commit, dialog cancel click, and toast navigation without crashes.
Link to Devin session: https://app.devin.ai/sessions/5d6b198dceb54bb2b8fa02c8a412f45c
Open in Devin Desktop: https://app.devin.ai/desktop/session/5d6b198dceb54bb2b8fa02c8a412f45c?variant=devin
Requested by: @tiensonqin