[WIKI-602] chore: disable image alignment tooltip for touch devices#7642
[WIKI-602] chore: disable image alignment tooltip for touch devices#7642sriramveeraghanta merged 2 commits intopreviewfrom
Conversation
WalkthroughAdds an isTouchDevice boolean prop to image alignment toolbar components and wires it through the toolbar root. Tooltips in alignment UI are disabled on touch devices. Introduces a CSS utility class (.touch-select-none) to prevent text selection, without modifying existing selectors. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Root as ImageToolbarRoot
participant Align as ImageAlignmentAction
participant TT as Tooltip(s)
User->>Root: Open image toolbar
Root->>Align: Render with isTouchDevice
alt isTouchDevice = true
Align->>TT: Render tooltips (disabled)
note right of TT: Tooltips disabled on touch
else isTouchDevice = false
Align->>TT: Render tooltips (enabled)
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Pull Request Linked with Plane Work Items Comment Automatically Generated by Plane |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/editor/src/core/extensions/custom-image/components/toolbar/alignment.tsx (1)
35-43: Add accessible names to icon-only buttons (tooltip isn’t a label).With tooltips disabled on touch, these icon-only buttons have no accessible name. Add
aria-labelto the trigger and each option for screen readers and general a11y. Also expose popup state on the trigger.<Tooltip disabled={isTouchDevice} tooltipContent="Align"> <button type="button" + aria-label="Image alignment" + aria-haspopup="menu" + aria-expanded={isDropdownOpen} className="h-full flex items-center gap-1 text-white/60 hover:text-white transition-colors" onClick={() => setIsDropdownOpen((prev) => !prev)} >- <Tooltip disabled={isTouchDevice} key={option.value} tooltipContent={option.label}> + <Tooltip disabled={isTouchDevice} key={option.value} tooltipContent={option.label}> <button type="button" + aria-label={option.label} className="flex-shrink-0 h-full grid place-items-center text-white/60 hover:text-white transition-colors" onClick={() => { handleChange(option.value); setIsDropdownOpen(false); }} > <option.icon className="size-3" /> </button> </Tooltip>Also applies to: 48-58
🧹 Nitpick comments (4)
packages/editor/src/styles/editor.css (1)
503-508: Consider suppressing iOS callout as well.Nice utility. To further reduce long-press distractions on iOS Safari, also set
-webkit-touch-callout: none;. Safe, small, and aligned with the intention here./* touch device styles */ .touch-select-none { user-select: none; -webkit-user-select: none; + -webkit-touch-callout: none; }packages/editor/src/core/extensions/custom-image/components/toolbar/root.tsx (2)
11-21: Prop threading looks good; consider a backward-compatible optional prop + local default.If any external caller renders
ImageToolbarRootwithoutisTouchDevice, making it optional avoids breakage while keeping runtime behavior intact by defaulting tofalse.type Props = { alignment: TCustomImageAlignment; editor: Editor; aspectRatio: number; downloadSrc: string; handleAlignmentChange: (alignment: TCustomImageAlignment) => void; height: string; - isTouchDevice: boolean; + isTouchDevice?: boolean; src: string; width: string; };Additionally (outside the changed range), default it locally:
// current line 24 const { alignment, editor, downloadSrc, handleAlignmentChange, isTouchDevice = false } = props;Also applies to: 45-46
33-38: Apply the new no-select utility on touch to the toolbar wrapper.Prevents accidental text selection when interacting with the toolbar on touch devices.
Example change outside the changed range:
<div className={cn( "absolute top-1 right-1 h-7 z-20 bg-black/80 rounded flex items-center gap-2 px-2 opacity-0 pointer-events-none group-hover/image-component:opacity-100 group-hover/image-component:pointer-events-auto transition-opacity", { "opacity-100 pointer-events-auto": shouldShowToolbar, }, isTouchDevice && "touch-select-none" )} >packages/editor/src/core/extensions/custom-image/components/toolbar/alignment.tsx (1)
35-43: Optional: use pointer events for snappier touch response.
onPointerDowntends to feel more responsive on touch thanonClick(which is delayed in some environments). If you switch, prevent default to avoid focus quirks.- onClick={() => setIsDropdownOpen((prev) => !prev)} + onPointerDown={(e) => { + e.preventDefault(); + setIsDropdownOpen((prev) => !prev); + }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
packages/editor/src/core/extensions/custom-image/components/toolbar/alignment.tsx(3 hunks)packages/editor/src/core/extensions/custom-image/components/toolbar/root.tsx(1 hunks)packages/editor/src/styles/editor.css(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Analyze (javascript)
- GitHub Check: Build and lint web apps
- GitHub Check: Build and lint web apps
🔇 Additional comments (4)
packages/editor/src/core/extensions/custom-image/components/toolbar/root.tsx (1)
11-21: ImageToolbarRootisTouchDeviceprop is correctly supplied at all call sitesAll inspected instances of
<ImageToolbarRoot … />now explicitly passisTouchDevice={isTouchDevice}, and the component itself definesisTouchDeviceas a required prop (no default), so there are no omissions to address.• In
packages/editor/src/core/extensions/custom-image/components/block.tsx(around theshowImageToolbarrender), the toolbar is instantiated with
… isTouchDevice={isTouchDevice} ….
• TheImageToolbarRootdefinition (packages/editor/src/core/extensions/custom-image/components/toolbar/root.tsx) declaresisTouchDevice: booleanon itsPropswith no fallback, enforcing explicit provision.No further changes are needed.
packages/editor/src/core/extensions/custom-image/components/toolbar/alignment.tsx (3)
34-44: LGTM: Tooltips are correctly gated off on touch devices.Disabling both the trigger tooltip and per-option tooltips when
isTouchDeviceis true matches the PR objective and avoids hover-only affordances on touch.Also applies to: 47-59
34-44: No changes needed fordisabledprop on TooltipThe
Tooltipcomponent inpackages/ui/src/tooltip/tooltip.tsxexplicitly defines and defaults adisabled?: booleanprop and forwards it to Blueprint’sTooltip2under the hood (see lines 28, 41, 78). Therefore, wrapping or conditional rendering around<Tooltip>solely to gatedisabledisn’t necessary.– packages/ui/src/tooltip/tooltip.tsx:
• ITooltipProps includesdisabled?: boolean
• Defaultdisabled = falsein component signature
•<Tooltip2 disabled={disabled} … />in renderLikely an incorrect or invalid review comment.
13-15: No SSR hydration risk inisTouchDevice—it's entirely consumer-supplied
The editor library itself does not computeisTouchDevicefromwindowor user-agent at render time—it’s declared as an optional prop (defaulting tofalse) and simply propagated through extensions and components. Any logic to detect touch capability must live in the embedding application, which should handle SSR vs. client-only detection. No changes are needed here.
3 participants
Description
This PR disables the tooltip for the image alignment action for touch devices.
Type of Change
Summary by CodeRabbit