[WIKI-524] fix: emoji fall back image #7354
Conversation
WalkthroughA custom local Emoji extension for the TipTap editor was introduced, handling emoji insertion, rendering, and suggestions. The extension replaces the previous external emoji integration, updates suggestion filtering, and incorporates emoji support into the read-only editor configuration. Serialization logic was adjusted for consistency, with no changes to public APIs. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Editor
participant EmojiExtension
participant SuggestionPlugin
User->>Editor: Types ":" + shortcode
Editor->>EmojiExtension: Detects input rule
EmojiExtension->>Editor: Inserts emoji node
User->>Editor: Types or pastes Unicode emoji
Editor->>EmojiExtension: Detects Unicode emoji in text
EmojiExtension->>Editor: Replaces with emoji node
User->>Editor: Types ":"
Editor->>SuggestionPlugin: Triggers emoji suggestion
SuggestionPlugin->>EmojiExtension: Fetches filtered emoji list
User->>SuggestionPlugin: Selects emoji
SuggestionPlugin->>Editor: Inserts emoji node and space
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (1)
✨ Finishing Touches
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
VipinDevelops
changed the title
|
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: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/editor/src/core/extensions/emoji/emoji.ts(1 hunks)packages/editor/src/core/extensions/emoji/extension.ts(2 hunks)packages/editor/src/core/extensions/emoji/suggestion.ts(1 hunks)packages/editor/src/core/extensions/read-only-extensions.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/editor/src/core/extensions/read-only-extensions.ts (1)
packages/editor/src/core/extensions/emoji/extension.ts (1)
EmojiExtension(8-30)
🪛 Biome (1.9.4)
packages/editor/src/core/extensions/emoji/emoji.ts
[error] 155-155: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
⏰ 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). (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
packages/editor/src/core/extensions/read-only-extensions.ts (1)
36-36: LGTM!The integration of the local
EmojiExtensioninto the read-only editor extensions is properly implemented.Also applies to: 77-77
packages/editor/src/core/extensions/emoji/extension.ts (1)
1-31: LGTM!The migration to the local Emoji implementation is well-executed, maintaining the existing serialization logic while improving code consistency.
| const supportMap: Record<number, boolean> = removeDuplicates(emojis.map((item) => item.version)) | ||
| .filter((version) => typeof version === "number") | ||
| .reduce((versions, version) => { | ||
| const emoji = emojis.find((item) => item.version === version && item.emoji); | ||
|
|
||
| return { | ||
| ...versions, | ||
| [version as number]: emoji ? isEmojiSupported(emoji.emoji as string) : false, | ||
| }; | ||
| }, {}); |
There was a problem hiding this comment.
Optimize the performance of emoji version support mapping
The spread syntax in the reducer causes O(n²) time complexity. Use a more efficient approach.
Apply this optimization:
const supportMap: Record<number, boolean> = removeDuplicates(emojis.map((item) => item.version))
.filter((version) => typeof version === "number")
- .reduce((versions, version) => {
+ .reduce((versions, version) => {
const emoji = emojis.find((item) => item.version === version && item.emoji);
-
- return {
- ...versions,
- [version as number]: emoji ? isEmojiSupported(emoji.emoji as string) : false,
- };
+ versions[version as number] = emoji ? isEmojiSupported(emoji.emoji as string) : false;
+ return versions;
}, {});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const supportMap: Record<number, boolean> = removeDuplicates(emojis.map((item) => item.version)) | |
| .filter((version) => typeof version === "number") | |
| .reduce((versions, version) => { | |
| const emoji = emojis.find((item) => item.version === version && item.emoji); | |
| return { | |
| ...versions, | |
| [version as number]: emoji ? isEmojiSupported(emoji.emoji as string) : false, | |
| }; | |
| }, {}); | |
| const supportMap: Record<number, boolean> = removeDuplicates( | |
| emojis.map((item) => item.version) | |
| ) | |
| .filter((version) => typeof version === "number") | |
| .reduce((versions, version) => { | |
| const emoji = emojis.find( | |
| (item) => item.version === version && item.emoji | |
| ); | |
| versions[version as number] = emoji | |
| ? isEmojiSupported(emoji.emoji as string) | |
| : false; | |
| return versions; | |
| }, {}); |
🧰 Tools
🪛 Biome (1.9.4)
[error] 155-155: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
🤖 Prompt for AI Agents
In packages/editor/src/core/extensions/emoji/emoji.ts around lines 149 to 158,
the reducer uses spread syntax inside the loop, causing O(n²) complexity. To fix
this, initialize the accumulator as an empty object and directly assign
properties to it within the reducer instead of spreading. This will improve
performance by avoiding repeated object copying.
| // const isSupported = this.storage.isSupported(emojiItem); | ||
| // const hasEmoji = !!emojiItem?.emoji; | ||
| // const hasFallbackImage = !!emojiItem?.fallbackImage; | ||
|
|
||
| const renderFallbackImage = false; | ||
|
|
||
| return [ | ||
| "span", | ||
| attributes, | ||
| renderFallbackImage | ||
| ? [ | ||
| "img", | ||
| { | ||
| src: emojiItem.fallbackImage, | ||
| draggable: "false", | ||
| loading: "lazy", | ||
| align: "absmiddle", | ||
| }, | ||
| ] | ||
| : emojiItem.emoji || `:${emojiItem.shortcodes[0]}:`, | ||
| ]; |
There was a problem hiding this comment.
Enable fallback image rendering to align with PR objectives
The fallback image rendering is hardcoded to false, which defeats the purpose of this PR ("fix: emoji fall back image"). The commented code suggests the proper implementation was started but not completed.
Implement the fallback image logic:
- // const isSupported = this.storage.isSupported(emojiItem);
- // const hasEmoji = !!emojiItem?.emoji;
- // const hasFallbackImage = !!emojiItem?.fallbackImage;
+ const isSupported = this.storage.isSupported(emojiItem);
+ const hasEmoji = !!emojiItem?.emoji;
+ const hasFallbackImage = !!emojiItem?.fallbackImage;
- const renderFallbackImage = false;
+ const renderFallbackImage =
+ (this.options.forceFallbackImages && !hasEmoji) ||
+ (this.options.forceFallbackImages && hasFallbackImage) ||
+ (this.options.forceFallbackImages && !isSupported && hasFallbackImage) ||
+ ((!isSupported || !hasEmoji) && hasFallbackImage);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // const isSupported = this.storage.isSupported(emojiItem); | |
| // const hasEmoji = !!emojiItem?.emoji; | |
| // const hasFallbackImage = !!emojiItem?.fallbackImage; | |
| const renderFallbackImage = false; | |
| return [ | |
| "span", | |
| attributes, | |
| renderFallbackImage | |
| ? [ | |
| "img", | |
| { | |
| src: emojiItem.fallbackImage, | |
| draggable: "false", | |
| loading: "lazy", | |
| align: "absmiddle", | |
| }, | |
| ] | |
| : emojiItem.emoji || `:${emojiItem.shortcodes[0]}:`, | |
| ]; | |
| const isSupported = this.storage.isSupported(emojiItem); | |
| const hasEmoji = !!emojiItem?.emoji; | |
| const hasFallbackImage = !!emojiItem?.fallbackImage; | |
| const renderFallbackImage = | |
| (this.options.forceFallbackImages && !hasEmoji) || | |
| (this.options.forceFallbackImages && hasFallbackImage) || | |
| (this.options.forceFallbackImages && !isSupported && hasFallbackImage) || | |
| ((!isSupported || !hasEmoji) && hasFallbackImage); | |
| return [ | |
| "span", | |
| attributes, | |
| renderFallbackImage | |
| ? [ | |
| "img", | |
| { | |
| src: emojiItem.fallbackImage, | |
| draggable: "false", | |
| loading: "lazy", | |
| align: "absmiddle", | |
| }, | |
| ] | |
| : emojiItem.emoji || `:${emojiItem.shortcodes[0]}:`, | |
| ]; |
🤖 Prompt for AI Agents
In packages/editor/src/core/extensions/emoji/emoji.ts between lines 194 and 214,
the fallback image rendering is disabled by hardcoding renderFallbackImage to
false. To fix this, uncomment and use the existing logic to determine if the
fallback image should be rendered by checking if emojiItem has a fallbackImage
and if the storage supports it. Replace the hardcoded false with this condition
so the fallback image renders correctly when appropriate.
Description
Fixes the issue of Emojis on the published page are significantly enlarged
Type of Change
Screenshots and Media (if applicable)
Test Scenarios
References
Summary by CodeRabbit
New Features
Improvements