-
Notifications
You must be signed in to change notification settings - Fork 16.6k
feat(EmojiTextArea): add Slack-like emoji autocomplete component #36417
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
CodeAnt AI is reviewing your PR. |
Code Review Agent Run #7ef3e7Actionable Suggestions - 0Additional Suggestions - 4
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
| * specific language governing permissions and limitations | ||
| * under the License. | ||
| */ | ||
| import { useState } from 'react'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: The file references React.FormEvent in the handleSubmit parameter but only imports useState from 'react' (no React namespace). This will cause a TypeScript "React is not defined" type error (or require a separate import) because the React identifier is used in a type position without being imported. Import the React namespace alongside useState so the React type is available. [type error]
Severity Level: Minor
| import { useState } from 'react'; | |
| import React, { useState } from 'react'; |
Why it matters? ⭐
The story uses the React namespace in a type position: handleSubmit = (e: React.FormEvent) => { ... }.
Without importing React (or importing the FormEvent type directly), TypeScript will complain that React is not defined.
This is a real type error, not a style nit. Adding import React, { useState } from 'react'; or import type { FormEvent } from 'react' and adjusting the signature fixes it.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset-frontend/packages/superset-ui-core/src/components/EmojiTextArea/EmojiTextArea.stories.tsx
**Line:** 19:19
**Comment:**
*Type Error: The file references `React.FormEvent` in the `handleSubmit` parameter but only imports `useState` from 'react' (no `React` namespace). This will cause a TypeScript "React is not defined" type error (or require a separate import) because the `React` identifier is used in a type position without being imported. Import the React namespace alongside `useState` so the `React` type is available.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.| test('filterEmojis returns matching emojis by shortcode', () => { | ||
| const results = filterEmojis('smile'); | ||
| expect(results.length).toBeGreaterThan(0); | ||
| expect(results[0].shortcode).toBe('smile'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: Fragile assertion: the test assumes the first returned match for "smile" will always be the exact smile shortcode. If ordering changes (or other shortcodes that include "smile" appear earlier), this assertion will be flaky or fail. Replace the strict index-based check with an existence check that the results include an item with shortcode smile. [possible bug]
Severity Level: Critical 🚨
| expect(results[0].shortcode).toBe('smile'); | |
| // Ensure the results include the exact 'smile' shortcode (don't rely on ordering) | |
| expect(results.some(e => e.shortcode === 'smile')).toBe(true); |
Why it matters? ⭐
The current assertion relies on ordering (results[0]) which can be flaky if the
filter implementation or emoji ordering changes. The proposed change (asserting
that some result has shortcode 'smile') correctly preserves the intent of the test
without depending on order, and it fixes a real brittleness in the test.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset-frontend/packages/superset-ui-core/src/components/EmojiTextArea/EmojiTextArea.test.tsx
**Line:** 73:73
**Comment:**
*Possible Bug: Fragile assertion: the test assumes the first returned match for "smile" will always be the exact `smile` shortcode. If ordering changes (or other shortcodes that include "smile" appear earlier), this assertion will be flaky or fail. Replace the strict index-based check with an existence check that the results include an item with shortcode `smile`.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.| expect(results1.length).toBe(results2.length); | ||
| expect(results1[0].shortcode).toBe(results2[0].shortcode); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: Fragile case-insensitivity test: the test compares lengths and the first element of the two result arrays, which can fail if order differs even when the sets are the same. Compare the sets of shortcodes in a deterministic way (sort them) to assert case-insensitive behavior robustly. [possible bug]
Severity Level: Critical 🚨
| expect(results1.length).toBe(results2.length); | |
| expect(results1[0].shortcode).toBe(results2[0].shortcode); | |
| // Compare shortcode sets in a deterministic way rather than relying on array order | |
| const sc1 = results1.map(r => r.shortcode).sort(); | |
| const sc2 = results2.map(r => r.shortcode).sort(); | |
| expect(sc1).toEqual(sc2); |
Why it matters? ⭐
Comparing lengths and the first element is brittle because arrays can be the same
set but in different order. Comparing deterministic sets (e.g., sorted shortcodes)
accurately verifies case-insensitivity and is a small, meaningful improvement.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset-frontend/packages/superset-ui-core/src/components/EmojiTextArea/EmojiTextArea.test.tsx
**Line:** 93:94
**Comment:**
*Possible Bug: Fragile case-insensitivity test: the test compares lengths and the first element of the two result arrays, which can fail if order differs even when the sets are the same. Compare the sets of shortcodes in a deterministic way (sort them) to assert case-insensitive behavior robustly.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.| if (!searchText) return []; | ||
|
|
||
| const lowerSearch = searchText.toLowerCase(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: Normalization bug: the function does not trim input or remove leading colon(s) (e.g. ":smile") which is commonly provided by Slack-like autocomplete triggers, so searches with a leading ':' or surrounding whitespace will not match any shortcodes; normalize the input (trim and strip leading colons) before searching. [logic error]
Severity Level: Minor
| if (!searchText) return []; | |
| const lowerSearch = searchText.toLowerCase(); | |
| const normalized = searchText?.trim().replace(/^:+/, ''); | |
| if (!normalized) return []; | |
| const lowerSearch = normalized.toLowerCase(); |
Why it matters? ⭐
This is a real usability bug: users commonly type ":smile" (leading colon) or add surrounding whitespace when using Slack-like autocompletes. As shortcodes in EMOJI_DATA don't include the colon, the current code won't match those inputs. Normalizing (trim + strip leading colons) directly fixes the behavior with a small, low-risk change.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset-frontend/packages/superset-ui-core/src/components/EmojiTextArea/emojiData.ts
**Line:** 559:561
**Comment:**
*Logic Error: Normalization bug: the function does not trim input or remove leading colon(s) (e.g. ":smile") which is commonly provided by Slack-like autocomplete triggers, so searches with a leading ':' or surrounding whitespace will not match any shortcodes; normalize the input (trim and strip leading colons) before searching.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.| ref, | ||
| ) => { | ||
| const [options, setOptions] = useState< | ||
| Array<{ value: string; label: React.ReactNode }> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: The options state is typed as objects with only value and label, but the code stores a data property on each option for later retrieval. This type mismatch can cause type-checking issues and makes the intended shape unclear; extend the state type to include an optional data?: EmojiItem property so the stored emoji item is typed and available. [type error]
Severity Level: Minor
| Array<{ value: string; label: React.ReactNode }> | |
| Array<{ value: string; label: React.ReactNode; data?: EmojiItem }> |
Why it matters? ⭐
The code stores a data field on each option object (used on select), but the useState type only declares value and label. That mismatch can cause TypeScript excess-property or type confusion. Extending the state type to include an optional data?: EmojiItem accurately models the stored shape and prevents type errors while making intent explicit.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset-frontend/packages/superset-ui-core/src/components/EmojiTextArea/index.tsx
**Line:** 78:78
**Comment:**
*Type Error: The `options` state is typed as objects with only `value` and `label`, but the code stores a `data` property on each option for later retrieval. This type mismatch can cause type-checking issues and makes the intended shape unclear; extend the state type to include an optional `data?: EmojiItem` property so the stored emoji item is typed and available.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.| // there's meaningful search text, allow the Enter to create newline | ||
| // This prevents accidental selection when typing something like: | ||
| // "let me show you an example:[Enter]" | ||
| if (timeSinceLastKey < 100 && lastSearchRef.current.length === 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: The Enter-key timing check is inverted: the code intends to treat a quick Enter after typing a meaningful emoji search as a newline (not a selection), but it checks for an empty search (length === 0) instead of a non-empty search. This will allow newline only when there's no search text and prevent the intended newline behavior when there actually is search text, causing accidental emoji selection or wrong newline handling. Change the condition to check for a non-empty search string. [logic error]
Severity Level: Minor
| if (timeSinceLastKey < 100 && lastSearchRef.current.length === 0) { | |
| if (timeSinceLastKey < 100 && lastSearchRef.current.length > 0) { |
Why it matters? ⭐
The comment and intent in the surrounding code say we should treat a very quick Enter after typing a meaningful search as a newline (to avoid accidental selection). The current condition checks for length === 0 (empty search), which is the opposite of the described intent. This is a real logic bug that can cause accidental emoji selection or wrong newline handling. Flipping to length > 0 aligns code with the documented intent and fixes the runtime behavior.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset-frontend/packages/superset-ui-core/src/components/EmojiTextArea/index.tsx
**Line:** 192:192
**Comment:**
*Logic Error: The Enter-key timing check is inverted: the code intends to treat a quick Enter after typing a meaningful emoji search as a newline (not a selection), but it checks for an empty search (length === 0) instead of a non-empty search. This will allow newline only when there's no search text and prevent the intended newline behavior when there actually is search text, causing accidental emoji selection or wrong newline handling. Change the condition to check for a non-empty search string.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Nitpicks 🔍
|
|
CodeAnt AI finished reviewing your PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR introduces a new EmojiTextArea component that provides Slack-like emoji autocomplete functionality using the Ant Design Mentions component as its foundation. The component triggers emoji suggestions when users type a colon followed by 2+ characters, with smart detection to avoid false positives from URLs.
Key changes:
- New React component with TypeScript support for emoji autocomplete in textareas
- Curated dataset of 400+ emojis with shortcodes and keyword-based search
- Comprehensive Storybook documentation demonstrating various usage patterns
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
superset-frontend/packages/superset-ui-core/src/components/index.ts |
Exports the new EmojiTextArea component and its utilities |
superset-frontend/packages/superset-ui-core/src/components/EmojiTextArea/index.tsx |
Main component implementation with validation logic and keyboard handling |
superset-frontend/packages/superset-ui-core/src/components/EmojiTextArea/emojiData.ts |
Emoji dataset with 400+ entries and filtering utility function |
superset-frontend/packages/superset-ui-core/src/components/EmojiTextArea/EmojiTextArea.test.tsx |
Unit tests for component rendering and emoji data utilities |
superset-frontend/packages/superset-ui-core/src/components/EmojiTextArea/EmojiTextArea.stories.tsx |
Storybook examples demonstrating component features and behavior |
| <div style={{ maxWidth: 600 }}> | ||
| <h3>Slack-like Trigger Behavior</h3> | ||
| <p style={{ color: 'var(--ant-color-text-secondary)' }}> | ||
| The emoji picker mimics Slack's behavior. Try these examples: |
Copilot
AI
Dec 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HTML entity ' can be replaced with a regular apostrophe ' in JSX strings for better readability. JSX automatically handles apostrophes in strings.
| The emoji picker mimics Slack's behavior. Try these examples: | |
| The emoji picker mimics Slack's behavior. Try these examples: |
| { shortcode: 'speech_balloon', emoji: '💬', keywords: ['talk', 'chat'] }, | ||
| { shortcode: 'thought_balloon', emoji: '💭', keywords: ['think', 'idea'] }, | ||
| { shortcode: 'zzz', emoji: '💤', keywords: ['sleep', 'tired'] }, | ||
| { shortcode: 'wave_emoji', emoji: '🌊', keywords: ['ocean', 'water'] }, |
Copilot
AI
Dec 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The shortcode wave_emoji is confusing because there's already a wave emoji (👋) on line 120. This ocean wave emoji should have a more distinct name like ocean_wave or water_wave to avoid confusion and to match common emoji naming conventions (e.g., Slack uses ocean for this emoji).
| { shortcode: 'wave_emoji', emoji: '🌊', keywords: ['ocean', 'water'] }, | |
| { shortcode: 'ocean_wave', emoji: '🌊', keywords: ['ocean', 'water'] }, |
| filterEmojis, | ||
| EMOJI_DATA, |
Copilot
AI
Dec 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Exporting filterEmojis and EMOJI_DATA from the main components index may expose internal implementation details. Consider whether these utilities need to be part of the public API. If they're only meant for internal use within the EmojiTextArea component, they could remain private. If external consumers need them, ensure they're properly documented as part of the public API contract.
| filterEmojis, | |
| EMOJI_DATA, |
| (text: string, props: MentionsProps): boolean => { | ||
| // Get the full value to check what precedes the colon | ||
| const fullValue = (props.value as string) || ''; |
Copilot
AI
Dec 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The validateSearch function receives props parameter but never uses it correctly. The function accesses props.value, but in React component callbacks, the component's current value should come from the component's own state or props, not from a passed-in props object. This could lead to stale or incorrect validation results.
The Ant Design Mentions component's validateSearch callback signature may not actually pass props as the second parameter. You should verify the actual API and either remove this unused parameter or properly integrate with the Mentions component's validation mechanism.
|
|
||
| const MIN_CHARS_BEFORE_POPUP = 2; | ||
|
|
||
| // Regex to match emoji characters (simplified, covers most common emojis) | ||
| const EMOJI_REGEX = | ||
| /[\u{1F300}-\u{1F9FF}]|[\u{2600}-\u{26FF}]|[\u{2700}-\u{27BF}]|[\u{1F600}-\u{1F64F}]|[\u{1F680}-\u{1F6FF}]|[\u{1F1E0}-\u{1F1FF}]/u; |
Copilot
AI
Dec 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The emoji regex is quite limited and described as "simplified". It misses many emoji ranges including:
- Emoji modifiers and skin tones (U+1F3FB-1F3FF)
- Combined emojis with ZWJ (Zero Width Joiner)
- Emoji presentation selectors
- Regional indicators beyond U+1F1E0-1F1FF
Consider using a more comprehensive emoji detection library or extending this regex to cover more Unicode emoji ranges for better compatibility.
| const MIN_CHARS_BEFORE_POPUP = 2; | |
| // Regex to match emoji characters (simplified, covers most common emojis) | |
| const EMOJI_REGEX = | |
| /[\u{1F300}-\u{1F9FF}]|[\u{2600}-\u{26FF}]|[\u{2700}-\u{27BF}]|[\u{1F600}-\u{1F64F}]|[\u{1F680}-\u{1F6FF}]|[\u{1F1E0}-\u{1F1FF}]/u; | |
| import emojiRegex from 'emoji-regex'; | |
| const MIN_CHARS_BEFORE_POPUP = 2; | |
| // Comprehensive regex to match all emoji characters, including modifiers and ZWJ sequences | |
| const EMOJI_REGEX = emojiRegex(); |
| const [options, setOptions] = useState< | ||
| Array<{ value: string; label: React.ReactNode }> | ||
| >([]); |
Copilot
AI
Dec 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The options type could be extracted to an interface or type alias for better maintainability and reusability. The inline type Array<{ value: string; label: React.ReactNode }> is used in multiple places and should be centralized. Consider creating an EmojiOption type that includes the optional data property mentioned in comments.
| // there's meaningful search text, allow the Enter to create newline | ||
| // This prevents accidental selection when typing something like: | ||
| // "let me show you an example:[Enter]" | ||
| if (timeSinceLastKey < 100 && lastSearchRef.current.length === 0) { |
Copilot
AI
Dec 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Enter key handling logic appears to have inverted logic. The condition checks if timeSinceLastKey < 100 AND lastSearchRef.current.length === 0, which means it only prevents selection when there's NO search text. This seems backwards - you'd want to prevent accidental selection when the user IS typing (has search text), not when they aren't.
The comment says "If typed very quickly (< 100ms since last keypress) and there's meaningful search text", but the code checks for length === 0 (no search text). This will allow accidental selection when typing :sm[Enter] quickly, which is the exact scenario you're trying to prevent.
| if (timeSinceLastKey < 100 && lastSearchRef.current.length === 0) { | |
| if (timeSinceLastKey < 100 && lastSearchRef.current.length > 0) { |
| label: ( | ||
| <span> | ||
| <span style={{ marginRight: 8 }}>{item.emoji}</span> | ||
| <span style={{ color: 'var(--ant-color-text-secondary)' }}> |
Copilot
AI
Dec 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The inline styles use CSS variable references (var(--ant-color-text-secondary)) which may not be defined in all contexts where this component is used. Consider using themed colors from the component library or documenting this dependency on Ant Design CSS variables.
| <span style={{ color: 'var(--ant-color-text-secondary)' }}> | |
| <span style={{ color: '#8c8c8c' }}> |
| // Store the full item for onSelect callback | ||
| data: item, |
Copilot
AI
Dec 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment mentions storing the full item for onSelect callback, but TypeScript doesn't recognize the data property on the option type. Consider explicitly typing the options array to include the data property, or document this as an extended property pattern.
| // Find where this search text starts in the full value | ||
| // The search text is what comes after the `:` prefix | ||
| const colonIndex = fullValue.lastIndexOf(`:${text}`); | ||
|
|
Copilot
AI
Dec 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using lastIndexOf to find the colon position could be inefficient and may find the wrong colon if there are multiple colons in the text. For example, if the text is :smile: :heart:, this will find the second colon, not necessarily the one being actively typed. Consider using a more precise method to locate the current cursor position and the colon being typed.
| // Find where this search text starts in the full value | |
| // The search text is what comes after the `:` prefix | |
| const colonIndex = fullValue.lastIndexOf(`:${text}`); | |
| // Find the colon that starts the current search, based on cursor position | |
| // Use selectionStart if available, otherwise fallback to end of text | |
| const cursorPos = | |
| typeof (props as any).selectionStart === 'number' | |
| ? (props as any).selectionStart | |
| : fullValue.length; | |
| // Search backward from the cursor for the nearest colon | |
| let colonIndex = -1; | |
| for (let i = cursorPos - text.length - 1; i >= 0; i--) { | |
| if (fullValue[i] === ':') { | |
| // Check if the substring after this colon matches the search text | |
| if (fullValue.slice(i + 1, i + 1 + text.length) === text) { | |
| colonIndex = i; | |
| break; | |
| } | |
| } | |
| } |
|
So many committable suggestions from the bot army! Pretty neat. |
Introduces a new EmojiTextArea component with Slack-like emoji autocomplete
behavior:
- Triggers on `:` prefix with 2+ character minimum (configurable)
- Smart trigger detection: colon must be preceded by whitespace, start of
text, or another emoji (prevents false positives like URLs)
- Prevents accidental Enter key selection when typing quickly
- Includes 400+ curated emojis with shortcodes and keyword search
- Fully typed with TypeScript, includes tests and Storybook stories
Usage:
```tsx
<EmojiTextArea
placeholder="Type 😄 to add emojis..."
onChange={(text) => console.log(text)}
minCharsBeforePopup={2}
/>
```
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
3b5573b to
e9965ab
Compare
|
CodeAnt AI is running Incremental review |
User description
Summary
Introduces a new
EmojiTextAreacomponent with Slack-like emoji autocomplete behavior for use in text input areas throughout Superset.Key Features:
:prefix with 2+ character minimum (configurable viaminCharsBeforePopup)http://example.com)Usage:
Trigger Behavior (Slack-like):
:smhello :sm😀:smhello:sm:sBEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Component demonstrated in Storybook - see
EmojiTextArea.stories.tsxTESTING INSTRUCTIONS
npm run test -- --testPathPatterns=EmojiTextAreanpm run storybookand navigate to Components > EmojiTextArea:smto see emoji suggestionshello:sm(no space before colon)ADDITIONAL INFORMATION
🤖 Generated with Claude Code
CodeAnt-AI Description
Add Slack-like emoji autocomplete to textareas
What Changed
Impact
✅ Easier emoji insertion in textareas✅ Fewer accidental emoji selections on Enter✅ More accurate emoji suggestions via shortcode and keyword search💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.