Custom emoji picker performance improvement #1408
Conversation
WalkthroughThe pull request introduces modifications to two files in the design system and conversation features. In the Changes
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
features/conversation/conversation-message/conversation-message-context-menu/conversation-message-context-menu-emoji-picker/conversation-message-context-menu-emoji-picker.tsx (2)
90-93: Consider adding a comment explaining the debounce delay choice.The increase to 300ms is a good balance between responsiveness and performance. However, it would be helpful to add a comment explaining why this specific value was chosen.
debounce((value: string) => { const filtered = filterEmojis(value); setFilteredReactions(filtered); setHasInput(value.length > 0); + // 300ms provides optimal balance between responsiveness and performance }, 300),
89-94: Consider adding cleanup for debounced function.To prevent potential memory leaks, consider cleaning up the debounced function when the component unmounts.
+ useEffect(() => { + return () => { + // Cleanup the debounced function + debouncedFilter.clear?.(); + }; + }, [debouncedFilter]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
design-system/Text/Text.presets.ts(1 hunks)features/conversation/conversation-message/conversation-message-context-menu/conversation-message-context-menu-emoji-picker/conversation-message-context-menu-emoji-picker.tsx(2 hunks)
🔇 Additional comments (3)
design-system/Text/Text.presets.ts (1)
35-35: Verify the impact on bottom sheet drawer titles.
The removal of textSizeStyles.xs from the bigBold preset makes it more consistent with its name. However, since this preset is used for bottom sheet drawer titles, we should verify the visual impact.
✅ Verification successful
The previous ast-grep pattern didn't work because it was looking for JSX/TSX syntax in a specific format. Let's try a broader search using ripgrep to find any usage of "bigBold" in the codebase, which will help us verify its usage in bottom sheet drawer titles or anywhere else.
Let's try a simpler ripgrep search without file type specifications since the tsx extension isn't recognized. This will help us find all occurrences of "bigBold" in the codebase.
The removal of textSizeStyles.xs from bigBold preset is safe
The search results confirm that the bigBold preset is indeed used in BottomSheetHeader.tsx for titles, along with several examples in Text.example.tsx. The typography configuration in theme/typography.ts shows that the bigBold style already has its own fontSize (24) and lineHeight (34) defined, which means removing textSizeStyles.xs from the preset is actually correcting a conflict where an extra-small size was being incorrectly mixed with a style meant to be big and bold.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for components using the bigBold preset in bottom sheet titles
ast-grep --pattern 'Text preset="bigBold"'
Length of output: 44
Script:
#!/bin/bash
# Search for usage of bigBold preset in TypeScript/JavaScript files
rg -l "bigBold" --type ts --type tsx --type js --type jsx
# Show the content of files containing "bigBold"
rg "bigBold" -C 3 --type ts --type tsx --type js --type jsx
Length of output: 173
Script:
#!/bin/bash
# Search for files containing "bigBold" using basic ripgrep
rg -l "bigBold"
# Show the content with context
rg "bigBold" -C 3
Length of output: 3975
features/conversation/conversation-message/conversation-message-context-menu/conversation-message-context-menu-emoji-picker/conversation-message-context-menu-emoji-picker.tsx (2)
49-56: Performance improvement in emoji filtering.
Good optimization! The changes improve performance by:
- Avoiding unnecessary slicing when input is empty
- Using a less strict threshold for matching, which provides better fuzzy search results
99-105: Good UX improvement with immediate reset.
Excellent change! The immediate reset when clearing input provides better user feedback, while still maintaining debounced filtering for non-empty inputs. This is a good balance between responsiveness and performance.
Performance Comparison ReportSignificant Changes To DurationThere are no entries Meaningless Changes To DurationShow entries
Show details
Render Count ChangesThere are no entries Render IssuesThere are no entries Added ScenariosThere are no entries Removed ScenariosThere are no entries |
Summary by CodeRabbit
bigBoldtext preset.