[WIKI-550] fix: emoji modal for touch device#7651
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughExpose Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Editor
participant EmojiExt as Emoji Extension
participant EmojiCore as Emoji Core (emoji.ts)
participant Utility as Utility Storage
User->>Editor: Trigger emoji flow (e.g., type ":")
Editor->>EmojiExt: use suggestion / init
EmojiExt->>EmojiCore: addProseMirrorPlugins()
EmojiCore->>Utility: getExtensionStorage(...).isTouchDevice
Utility-->>EmojiCore: isTouchDevice (true/false)
alt isTouchDevice == true
Note over EmojiCore: Return empty plugin list (plugins disabled)
EmojiCore-->>EmojiExt: []
else isTouchDevice == false
Note over EmojiCore: Return emoji plugins (suggestion enabled)
EmojiCore-->>EmojiExt: [emoji plugins...]
EmojiExt->>EmojiCore: uses `emojiSuggestion` (named import)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 |
There was a problem hiding this comment.
Pull Request Overview
This PR disables emoji suggestions on touch devices by checking for touch device status and preventing the emoji modal from appearing. The change improves the user experience on mobile and tablet devices where emoji suggestions may interfere with touch interactions.
- Adds touch device detection to disable emoji suggestions on touch-enabled devices
- Modifies the emoji suggestion onStart handler to exit early for touch devices
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/editor/src/core/extensions/emoji/suggestion.ts (2)
53-63: Optional: reuse utility storage and reduce duplicate lookupsMinor cleanup that also centralizes the storage reference:
return { onStart: (props: SuggestionProps): void => { - const isTouchDevice = !!getExtensionStorage(props.editor, CORE_EXTENSIONS.UTILITY).isTouchDevice; - if (!props.clientRect || isTouchDevice) return; - - editor = props.editor; - - // Track active dropdown - getExtensionStorage(editor, CORE_EXTENSIONS.UTILITY).activeDropbarExtensions.push(CORE_EXTENSIONS.EMOJI); + const utilityStorage = getExtensionStorage(props.editor, CORE_EXTENSIONS.UTILITY); + const isTouchDevice = !!utilityStorage.isTouchDevice; + if (!props.clientRect || isTouchDevice) return; + + editor = props.editor; + + // Track active dropdown + utilityStorage.activeDropbarExtensions.push(CORE_EXTENSIONS.EMOJI);
2-2: Nit: make Editor a type-only import to avoid bundling runtime symbol
Editoris only used as a type. Use a type-only import to keep it out of the runtime bundle.-import { ReactRenderer, Editor } from "@tiptap/react"; +import { ReactRenderer } from "@tiptap/react"; +import type { Editor } from "@tiptap/react";
📜 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 (1)
packages/editor/src/core/extensions/emoji/suggestion.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/editor/src/core/extensions/emoji/suggestion.ts (2)
packages/editor/src/core/helpers/get-extension-storage.ts (1)
getExtensionStorage(5-8)packages/editor/src/index.ts (1)
CORE_EXTENSIONS(24-24)
⏰ 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). (4)
- GitHub Check: Build and lint web apps
- GitHub Check: Analyze (javascript)
- GitHub Check: Build and lint web apps
- GitHub Check: Build and lint web apps
🔇 Additional comments (1)
packages/editor/src/core/extensions/emoji/suggestion.ts (1)
55-56: Utility storage includes and initializesisTouchDeviceVerified in packages/editor/src/core/extensions/utility.ts that the
UtilityExtensionStorageinterface declares anisTouchDevice: booleanfield, and the extension’s initializer (line 81) sets it from the incomingisTouchDeviceprop. No further changes needed.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/editor/src/core/extensions/emoji/suggestion.ts (1)
57-61: Resolved: editor is now assigned before storage access
editor = props.editorprecedes the storage push, fixing the earlier “editor is undefined” bug. Looks good.
🧹 Nitpick comments (3)
packages/editor/src/core/extensions/emoji/suggestion.ts (1)
13-13: Type source consistency (optional): import EmojiOptions from local extension to avoid driftYou’re importing
EmojiOptionsfrom@tiptap/extension-emojiwhile also defining a localEmojiOptionsinemoji.ts. To avoid type drift, consider a type-only import from the local file.-import type { EmojiOptions } from "@tiptap/extension-emoji"; +import type { EmojiOptions } from "./emoji";packages/editor/src/core/extensions/emoji/emoji.ts (2)
19-22: New imports are fine; add a defensive optional chain to avoid hard crash if UTILITY storage is absentIf the UTILITY extension isn’t registered (or storage not initialized yet), direct property access can throw. Defensive
?.costs nothing and avoids editor init crashes.- const isTouchDevice = !!getExtensionStorage(this.editor, CORE_EXTENSIONS.UTILITY).isTouchDevice; + const isTouchDevice = !!getExtensionStorage(this.editor, CORE_EXTENSIONS.UTILITY)?.isTouchDevice;
346-350: Clarify emoji plugin disablement scope on touch devicesI’ve confirmed that
isTouchDeviceis defined in the Utility extension storage (e.g.packages/editor/src/core/extensions/utility.ts) and used throughout the editor codebase. As it stands, the earlyreturn []inemoji.tson touch devices disables both:
- the Suggestion (typeahead) plugin, and
- the standalone “emoji” plugin (double-click handling + unicode-to-node conversion)
If the intention is only to suppress the emoji suggestion modal on touch while retaining the conversion/UX plugin, I recommend refactoring to conditionally include only the Suggestion plugin. For example:
--- a/packages/editor/src/core/extensions/emoji/emoji.ts +++ b/packages/editor/src/core/extensions/emoji/emoji.ts @@ -344,11 +344,23 @@ export default class Emoji extends Extension { addProseMirrorPlugins() { - const isTouchDevice = !!getExtensionStorage(this.editor, CORE_EXTENSIONS.UTILITY).isTouchDevice; - if (isTouchDevice) { - return []; - } - return [ - Suggestion({ /* … */ }), - new Plugin({ key: new PluginKey("emoji"), /* double-click & appendTransaction */ }), - ]; + const isTouchDevice = !!getExtensionStorage(this.editor, CORE_EXTENSIONS.UTILITY)?.isTouchDevice; + const plugins: Plugin[] = []; + + // Only disable the typeahead on touch; always include the core emoji plugin + if (!isTouchDevice) { + plugins.push( + Suggestion({ + editor: this.editor, + findSuggestionMatch: customFindSuggestionMatch, + ...this.options.suggestion, + }) + ); + } + + plugins.push( + new Plugin({ + key: new PluginKey("emoji"), + props: { + handleDoubleClickOn: (view, pos, node) => { /* … */ }, + }, + appendTransaction: (transactions, oldState, newState) => { /* … */ }, + }) + ); + + return plugins;— After this change:
- Touch devices: colon (
:) no longer opens the suggestion modal, but pasted/typed Unicode emojis still convert.- Desktop: both suggestion and conversion plugins remain active.
Let me know if this aligns with the product goal for touch–device behavior.
📜 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/emoji/emoji.ts(2 hunks)packages/editor/src/core/extensions/emoji/extension.ts(2 hunks)packages/editor/src/core/extensions/emoji/suggestion.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
packages/editor/src/core/extensions/emoji/emoji.ts (1)
packages/editor/src/core/helpers/get-extension-storage.ts (1)
getExtensionStorage(5-8)
packages/editor/src/core/extensions/emoji/extension.ts (1)
packages/editor/src/core/extensions/emoji/suggestion.ts (1)
emojiSuggestion(13-118)
packages/editor/src/core/extensions/emoji/suggestion.ts (1)
packages/editor/src/core/extensions/emoji/emoji.ts (1)
EmojiOptions(73-79)
⏰ 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). (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Build and lint web apps
🔇 Additional comments (2)
packages/editor/src/core/extensions/emoji/suggestion.ts (1)
13-13: emojiSuggestion named export migration is safeNo default imports of
./suggestionremain in the repo—only a single named import is used—so switching to a named export will not break any internal consumers. You can safely omit the temporaryexport default emojiSuggestioncompatibility layer.• Verified named import at
• packages/editor/src/core/extensions/emoji/extension.ts:6 —import { emojiSuggestion } from "./suggestion";packages/editor/src/core/extensions/emoji/extension.ts (1)
6-6: Import and wire-up to named export confirmedI’ve verified that there are no remaining default imports of
./suggestionin thepackages/editordirectory and that both occurrences (lines 6 and 28 ofextension.ts) now correctly use the named import:import { emojiSuggestion } from "./suggestion";No further changes are needed here—everything is wired up as intended.
Description
This PR check for isTouch device and disable emoji suggestion it.
Screenshots and Media (if applicable)
Test Scenarios
References
Summary by CodeRabbit