fix(composer): dismiss composer tooltips when cursor leaves the trigger#1272
Open
tellaho wants to merge 7 commits into
Open
fix(composer): dismiss composer tooltips when cursor leaves the trigger#1272tellaho wants to merge 7 commits into
tellaho wants to merge 7 commits into
Conversation
Toolbar tooltips rendered with pointer-events:auto, so while visible they sat over the message textarea and swallowed clicks meant for the editor beneath. Fix the click-through on the composer surface only — where labels are known-short — rather than promising it app-wide for every tooltip. Add ComposerIconButton (desktop/src/features/messages/ui): a forwardRef component owning the Tooltip -> Trigger -> Button -> Content shape that bakes pointer-events-none onto the tooltip content (the floating popup) only. The trigger keeps pointer/focus so focus-to-show still works (WCAG content-on-hover-or-focus). Because the button owns its tooltip, the override can't be forgotten and future composer icon buttons inherit it. Swap the 5 main-toolbar icon buttons (formatting toggle/close, mention, attach, spoiler) to it. The formatting sub-toolbar (Bold/Italic/lists/Quote) maps over raw <button>s with custom active-state styling, not the shared Button, so bake pointer-events-none onto its single TooltipContent there instead of restructuring to ComposerIconButton — keeping the override in one obvious place for that row. Shared tooltip.tsx is untouched. Adds composer-scoped e2e cases asserting both surfaces' tooltips are visible but click-through, and registers the spec in the smoke project. Co-authored-by: Taylor Ho <taylorkmho@gmail.com> Co-authored-by: npub1223z34h <52a228d6edf316ec6812ac3c9fc0d696ab59fc7954d77e7be31eedcddf91335b@sprout-oss.stage.blox.sqprod.co> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
2987fc4 to
80c7633
Compare
Collaborator
Author
|
moving down to draft because it's not behaving as expected |
The click-through fix (pointer-events-none) stopped the popup from swallowing clicks but left Radix Tooltips hover-to-persist safe bridge intact, so the cursor could slide off the trigger onto the popup, keep it alive, and select its text. Set disableHoverableContent on the composer Tooltip Roots (ComposerIconButton + the FormattingToolbar map) so these label tooltips dismiss the instant the pointer leaves the trigger. Scoped to the composer Roots only — the shared TooltipProvider and tooltip.tsx keep their app-wide defaults, and the trigger keeps its hover/focus-to-show lifecycle (WCAG content-on-hover-or-focus). Co-authored-by: Taylor Ho <taylorkmho@gmail.com> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
…ntent disableHoverableContent + pointer-events-none on the inner TooltipContent still let the tooltip camp: Radix wraps content in a positioned [data-radix-popper-content-wrapper] DIV it styles with pointer-events:auto and never exposes to props. That wrapper overlaps the trigger, so a real cursor sliding off the trigger lands on it, persists the tooltip, and can select text. The inner pointer-events-none was one level too shallow. Tag the composer TooltipContent with data-composer-tooltip and add a scoped globals.css rule that reaches the wrapper via :has(> [data-composer-tooltip]), setting pointer-events:none + user-select:none on the actual camp surface. Scoped to composer tooltips only — shared tooltip.tsx and the app-wide TooltipProvider untouched; trigger keeps hover/focus-to-show (WCAG content-on-hover-or-focus). Inner pointer-events-none/select-none kept as belt-and-suspenders. Co-authored-by: Taylor Ho <taylorkmho@gmail.com> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
…utes - Delete ComposerIconButton.tsx entirely — composer buttons use the base Tooltip/TooltipContent from shared/ui/tooltip directly again, and MessageComposerToolbar returns to main's structure - The click-through/anti-camp treatment is now two opt-in attributes at each call site: disableHoverableContent on the Tooltip root (drops Radix's hover-to-persist safe bridge) and data-composer-tooltip on TooltipContent (keys the scoped globals.css rule that kills pointer-events/user-select on the Radix popper wrapper) - Drop the pointer-events-none/select-none classes from TooltipContent — the wrapper rule's pointer-events:none inherits down to the content, verified by the e2e computed-style assertions - Opt in the emoji picker tooltip (ComposerEmojiPicker) — previously missed because it hand-rolls its tooltip around a nested PopoverTrigger, making it the one composer tooltip that still persisted after hover - Opt in the three attachment tooltips in ComposerAttachments (Remove attachment x2, Cancel upload) so the whole composer surface behaves consistently - Rewrite the globals.css rule comment to document the opt-in recipe instead of referencing the deleted component - e2e: add an emoji-picker tooltip test that also asserts the Radix popper wrapper computes pointer-events:none (the actual camp surface); update a stale ComposerIconButton comment reference Co-authored-by: Taylor Ho <taylorkmho@gmail.com> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
…leave - Remove the data-composer-tooltip marker from all composer TooltipContent call sites (MessageComposerToolbar, FormattingToolbar, ComposerEmojiPicker, ComposerAttachments) and delete the scoped [data-radix-popper-content-wrapper]:has() rule from globals.css - The tooltip-persistence bug (cursor sliding off the trigger onto the popup keeps it camped over the editor) is fully fixed by disableHoverableContent alone, which every composer tooltip keeps — the CSS rule only added click-through for the narrow exit-animation and keyboard-focus windows, judged not worth the extra mechanism - Update the toolbar comment to describe the single-attribute treatment - e2e: replace screenshot-tooltip-pointer-events.spec.ts (asserted computed pointer-events:none) with composer-tooltip-dismiss.spec.ts, which hovers each trigger (toolbar, formatting sub-toolbar, emoji picker), slides the cursor onto the tooltip popup, and asserts it dismisses; verified the emoji test fails when disableHoverableContent is removed, so the spec catches regressions - playwright.config.ts: register the renamed spec in the smoke project Co-authored-by: Taylor Ho <taylorkmho@gmail.com> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
- Remove comments referencing the superseded click-through approach so the spec reads on its own terms for fresh readers Co-authored-by: Taylor Ho <taylorkmho@gmail.com> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
Take main's AttachmentMediaLightbox extraction (#1380) for the composer attachment preview, then re-apply this PR's one-attribute change: add disableHoverableContent to the remove-attachment Tooltips so they dismiss when the cursor leaves the trigger. Co-authored-by: Taylor Ho <taylorkmho@gmail.com> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
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.
Category: fix
User Impact: Tooltips in the message composer now disappear as soon as the cursor moves off their button, instead of lingering over the message box.
Problem: Hovering a composer toolbar button (mention, attach, emoji, spoiler, formatting) shows a label tooltip that floats over the message textarea. Sliding the cursor off the button onto the tooltip kept it open indefinitely, leaving it camped over the editor.
Solution: Radix Tooltip's default "hoverable content" behavior keeps a tooltip open while the cursor is over the popup — right for interactive tooltips, wrong for these one-word labels. Every tooltip on the composer surface now sets
disableHoverableContent, so it dismisses the instant the pointer leaves the trigger. One attribute per call site; app-wide tooltip defaults are untouched.File changes
desktop/src/features/messages/ui/MessageComposerToolbar.tsx
The five toolbar tooltips (formatting toggle x2, close formatting, mention, attach, spoiler) set
disableHoverableContent. A comment above the first one explains why.desktop/src/features/messages/ui/ComposerEmojiPicker.tsx
Same attribute on the emoji button's tooltip.
desktop/src/features/messages/ui/FormattingToolbar.tsx
Same attribute on the Bold/Italic/lists/Quote sub-toolbar tooltips. These also render in the floating selection tray, which inherits the fix.
desktop/src/features/messages/ui/ComposerAttachments.tsx
Same attribute on the attachment-chip tooltips (Remove attachment, Cancel upload) so the whole composer surface behaves consistently.
desktop/tests/e2e/composer-tooltip-dismiss.spec.ts
New spec with three tests (toolbar button, formatting sub-toolbar, emoji picker): hover the trigger, slide the cursor onto the tooltip popup, and assert the tooltip dismisses.
desktop/playwright.config.ts
Registers the new spec in the smoke project.
Reproduction Steps
cd desktop && pnpm exec playwright test composer-tooltip-dismiss— 3 tests pass.