H-6529: fix click detection in entity editor - #9240
Open
claude[bot] wants to merge 2 commits into
Open
Conversation
The DataTypeSelector renders its option menu in a MUI Popper, which
portals to document.body – outside the DOM tree of the Glide Grid cell
editor that hosts it (EditorTypePicker, used by the entity editor's
value cell for properties with more than one permitted data type).
Glide Grid's ClickOutsideContainer registers a capture-phase mousedown
listener on document and finishes editing whenever the target is outside
the editor's subtree and has no ancestor carrying
GRID_CLICK_IGNORE_CLASS ("click-outside-ignore"). Pressing the mouse on
a data type option therefore unmounted the editor – and with it the
menu – before the option's click handler could run, so choosing a data
type appeared to do nothing.
Add GRID_CLICK_IGNORE_CLASS to the Popper, matching what
selector-autocomplete and the other portalled popups used inside grid
cell editors already do.
Follow-on to the previous commit, covering the rest of the click-detection defects of the same class in the entity editor's grid cell editors. Glide Grid's ClickOutsideContainer ends editing on any capture-phase mousedown whose target is neither inside the editor's subtree nor under an ancestor carrying GRID_CLICK_IGNORE_CLASS. Anything portalled to document.body therefore has to opt out explicitly. Stale global click-outside suppression (single-value-editor): ensureFormValidation adds GRID_CLICK_IGNORE_CLASS to document.body and attaches a document click listener, but only removed them inside validationHandler, which needs a later document click to run. Grid closes overlays on Escape and Tab without any click, so the editor could unmount with both still in place – disabling click-outside for every grid editor on the page (Glide walks parentElement up to body) and leaving a listener that calls onFinishedEditing from an unmounted editor's closure. The teardown is now held in a ref and run from a useEffect cleanup, so it happens on unmount too. Validation behaviour itself is unchanged. Unguarded MUI tooltips: tooltips are interactive by default, so their poppers keep pointer-events: auto and portal to document.body, making a mousedown on one read as a click outside the editor. Add disableInteractive and the ignore class to the four reachable from inside a grid cell editor, matching row-action.tsx: - DataTypeLabel's description tooltip, which wraps every option row in the data type menu - array-editor value-chip, boolean-input, readonly-grid-popup Also trims the comment added in the previous commit to the surrounding density, and updates the two stale H-1834 references to FE-244.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
github-actions
Bot
added
area/apps > hash*
PR SummaryCursor Bugbot is generating a summary for commit f60c23c. Configure here. |
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
2 participants
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.
Requested by Ciaran Morinan · Slack thread
🌟 What is the purpose of this PR?
Fixes click detection in the entity editor's cell editors.
Before. Clicking anything in the choose-data-type modal did nothing. Opening a property that accepts more than one data type showed the type list, but clicks on the options were dead — so there was no way to set or change a value's data type. The same underlying fault made other controls inside cell editors unreliable: an option's description tooltip, a value chip's type icon, a boolean checkbox, or the copy button on a read-only value could dismiss the editor mid-edit. And after editing a validated text or number field and closing the editor with Escape or Tab, click-outside stopped working for every cell editor on the page.
After. Clicks in the choose-data-type modal work — options select, and the data type is applied. Tooltips and controls inside cell editors no longer dismiss the editor. Editors closed with Escape or Tab clean up after themselves.
How. The grid closes a cell editor on any
mousedownoutside the editor's DOM subtree unless an ancestor of the event target carriesclick-outside-ignore. The data-type menu and several tooltips render in portals attached todocument.body, so they fell outside that subtree and were treated as outside clicks — the menu's mousedown unmounted the editor before the option's click handler could run. Adding that class (the convention the sibling popups in this area already follow) fixes it. Separately, the validation enforcer in the single-value editor added the same class todocument.bodyand a document-level click listener with no unmount cleanup; the teardown is now held in a ref and run from an effect cleanup, so it also runs on the Escape/Tab close path.🔗 Related links
🔍 What does this change?
data-type-selector.tsx— the data type option menu's popper, and the per-option description tooltip, now carry the ignore class, so pressing the mouse on an option no longer unmounts the editor hosting the menu.single-value-editor.tsx— the validation enforcer's teardown (removing the ignore class fromdocument.bodyand detaching its document click listener) is held in a ref and run from an effect cleanup, so it also happens when the editor unmounts via Escape or Tab rather than only on a later document click.value-chip.tsx,boolean-input.tsx,readonly-grid-popup.tsx— the tooltips reachable from inside a cell editor are made non-interactive and carry the ignore class, matching the existing treatment inrow-action.tsx.Pre-Merge Checklist 🚀
🚢 Has this modified a publishable library?
This PR:
@hashintel/design-systemisprivateand listed in.changeset/config.json'signore, so no changeset is needed.📜 Does this require a change to the docs?
The changes in this PR:
🕸️ Does this require a change to the Turbo Graph?
The changes in this PR:
Validation was limited, and this needs a reviewer to click through it.
tscandeslintcannot pass cleanly on these packages even onmain, because of an unbuildable Rust→wasm artifact. Rather than claim a clean run, both were run onmainand on this branch and the output compared: no new findings on the branch.oxfmt --checkpasses.🐾 Next steps
document.bodythat can be reached from inside a grid cell editor and are still missing the ignore class — this PR covers the ones reachable from the reported flows, not an exhaustive audit.🛡 What tests cover this?
None. This is DOM-portal and event-ordering behaviour inside Glide Grid's editor overlay, which the current suite does not exercise; no new tests are added here.
❓ How to test this?
📹 Demo
No demo — see the screen recording of the original bug on H-6529 (internal).