[WEB-4320] dev: propel emoji reaction component#7741
[WEB-4320] dev: propel emoji reaction component#7741sriramveeraghanta merged 11 commits intopreviewfrom
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. WalkthroughAdds new EmojiReaction UI components and an EmojiReactionPicker, exposes them via package exports and barrel index, and registers build entries. Introduces Storybook stories for both. Adjusts CommandList/CommandItem to forward all props. Adds a trailing newline to a Russian translations file. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant BTN as Trigger Button
participant POP as Popover
participant EMJ as EmojiRoot
participant P as Parent (consumer)
U->>BTN: Click
BTN->>POP: handleToggle(open)
POP-->>U: Panel opens
U->>EMJ: Select emoji
EMJ-->>P: onChange(emojiString)
alt closeOnSelect = true
EMJ->>POP: request close
POP-->>U: Panel closes
end
sequenceDiagram
autonumber
actor U as User
participant GRP as EmojiReactionGroup
participant ITM as EmojiReaction
participant BTN as EmojiReactionButton (+)
U->>ITM: Click existing reaction
ITM-->>GRP: onReactionClick(emoji)
Note over GRP: Parent handles count/ reacted state externally
U->>BTN: Click Add (+)
Note over BTN: Triggers parent flow to open picker (out of scope)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (1 passed, 2 warnings)❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
Poem
✨ 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. Comment |
|
Pull Request Linked with Plane Work Items Comment Automatically Generated by Plane |
There was a problem hiding this comment.
Pull Request Overview
This PR introduces an emoji reaction component system for Propel, enabling users to react to content with emojis and view reaction counts and user lists.
- Adds
EmojiReaction,EmojiReactionGroup, andEmojiReactionButtoncomponents with animated counters - Introduces
EmojiReactionPickercomponent for selecting emojis from a picker interface - Creates an
AnimatedCountercomponent to provide smooth count transitions in reactions
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/propel/tsdown.config.ts | Adds build configuration for new animated-counter and emoji-reaction components |
| packages/propel/package.json | Adds package exports for animated-counter and emoji-reaction modules |
| packages/propel/src/emoji-reaction/index.ts | Main export file for emoji reaction components and types |
| packages/propel/src/emoji-reaction/emoji-reaction.tsx | Core emoji reaction components with size variants and tooltip support |
| packages/propel/src/emoji-reaction/emoji-reaction.stories.tsx | Storybook stories demonstrating emoji reaction usage |
| packages/propel/src/emoji-reaction/emoji-reaction-picker.tsx | Emoji picker component with popover integration |
| packages/propel/src/emoji-reaction/emoji-reaction-picker.stories.tsx | Storybook stories for emoji picker functionality |
| packages/propel/src/animated-counter/index.ts | Export file for animated counter component |
| packages/propel/src/animated-counter/animated-counter.tsx | Animated counter with slide transitions for number changes |
| packages/propel/src/animated-counter/animated-counter.stories.tsx | Storybook stories demonstrating animated counter behavior |
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: 7
🧹 Nitpick comments (10)
packages/propel/src/animated-counter/animated-counter.tsx (3)
54-55: Avoid class vs inline animation conflicts. Use one source of truth.Both animate-[...] classes and inline style set animation, with style winning. Drop the redundant classes to reduce confusion.
- "animate-[slideOut_0.25s_ease-out_forwards]", + // animation driven via inline style below- isAnimating && "animate-[slideIn_0.25s_ease-out_forwards]", + // animation driven via inline style belowAlso applies to: 75-76
10-14: Prevent clipping/jitter for multi-digit counts; apply consumer styles consistently.Fixed w-* can clip 2–3 digit counts inside an overflow-hidden container. Also, apply className to the prev span for consistent theming. Tabular numerals reduce width jitter.
const sizeClasses = { - sm: "text-xs h-4 w-4", - md: "text-sm h-5 w-5", - lg: "text-base h-6 w-6", + sm: "text-xs h-4 min-w-[2ch]", + md: "text-sm h-5 min-w-[3ch]", + lg: "text-base h-6 min-w-[4ch]", };className={cn( - "absolute inset-0 flex items-center justify-center font-medium", + "absolute inset-0 flex items-center justify-center font-medium tabular-nums", /* animation class removed */, direction === "up" && "[--slide-out-dir:-100%]", direction === "down" && "[--slide-out-dir:100%]", - sizeClass, + sizeClass, + className, )}className={cn( - "flex items-center justify-center font-medium", + "flex items-center justify-center font-medium tabular-nums", /* animation class removed */, !isAnimating && "opacity-100", sizeClass, className )}Also applies to: 52-58, 73-79
47-47: Add live region for accessibility.Announce count updates to AT without extra noise.
- <div className={cn("relative inline-flex items-center justify-center overflow-hidden", sizeClass)}> + <div + role="status" + aria-live="polite" + aria-atomic="true" + className={cn("relative inline-flex items-center justify-center overflow-hidden", sizeClass)} + >packages/propel/src/emoji-reaction/emoji-reaction-picker.stories.tsx (1)
33-36: Nit: avoid unnecessary template literal for static className.- label={ - <span className={`flex items-center justify-center rounded-md px-2 size-8 text-xl`}> + label={ + <span className="flex items-center justify-center rounded-md px-2 size-8 text-xl">packages/propel/src/emoji-reaction/emoji-reaction.stories.tsx (1)
1-4: Remove unused imports.useState, EmojiReactionGroup, and EmojiReactionType aren’t used in this story.
-import { useState } from "react"; -import type { Meta, StoryObj } from "@storybook/react-vite"; -import { EmojiReaction, EmojiReactionGroup, EmojiReactionType } from "./emoji-reaction"; +import type { Meta, StoryObj } from "@storybook/react-vite"; +import { EmojiReaction } from "./emoji-reaction";packages/propel/src/emoji-reaction/emoji-reaction-picker.tsx (2)
8-22: Optional: add aria-label support for better a11y on the trigger.Helps when label is an icon-only element.
export interface EmojiReactionPickerProps { isOpen: boolean; handleToggle: (value: boolean) => void; buttonClassName?: string; closeOnSelect?: boolean; disabled?: boolean; dropdownClassName?: string; label: React.ReactNode; + ariaLabel?: string; onChange: (emoji: string) => void; placement?: TPlacement; searchDisabled?: boolean; searchPlaceholder?: string; side?: TSide; align?: TAlign; } @@ const { isOpen, handleToggle, buttonClassName, closeOnSelect = true, disabled = false, dropdownClassName, label, + ariaLabel, onChange, placement, searchDisabled = false, searchPlaceholder = "Search", side = "bottom", align = "start", } = props; @@ - <Popover.Button className={cn("outline-none", buttonClassName)} disabled={disabled}> + <Popover.Button + className={cn("outline-none", buttonClassName)} + disabled={disabled} + aria-label={ariaLabel || (typeof label === "string" ? label : undefined)} + > {label} </Popover.Button>Also applies to: 24-39, 59-63
61-69: Nit: keep cn import path consistent with other modules.Other components import cn from "../utils". Consider aligning for consistency.
packages/propel/src/emoji-reaction/emoji-reaction.tsx (3)
66-75: Optional: compute displayEmoji once and reuse; add aria-pressed for toggled state.Reduces repeated work and improves accessibility for “reacted” state.
@@ ) => { const sizeClass = sizeClasses[size]; + const displayEmoji = React.useMemo(() => { + const converted = stringToEmoji(emoji); + return converted || emoji; + }, [emoji]); + const handleClick = () => { onReactionClick?.(emoji); }; @@ - <button + <button ref={ref} onClick={handleClick} + aria-pressed={reacted} className={cn( @@ - <span className={sizeClass.emoji}>{emoji}</span> - {showCount && count > 0 && <AnimatedCounter count={count} size={size} className={sizeClass.count} />} + <span className={sizeClass.emoji}>{displayEmoji}</span> + {showCount && count > 0 && ( + <AnimatedCounter count={count} size={size} className={sizeClass.count} /> + )}Also applies to: 94-111
162-171: Optional: avoid index in list keys if possible.If emojis are unique within the group, prefer key={reaction.emoji}. Index can cause unnecessary remounts when items reorder.
- {reactions.map((reaction, index) => ( + {reactions.map((reaction, index) => ( <EmojiReaction - key={`${reaction.emoji}-${index}`} + key={reaction.emoji ?? index}
127-143: Nit: add aria-label to the add reaction button.Title is present; aria-label helps SR users.
- <button + <button ref={ref} onClick={onAddReaction} className={cn( @@ - title="Add reaction" + title="Add reaction" + aria-label="Add reaction" {...props} >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
packages/propel/package.json(1 hunks)packages/propel/src/animated-counter/animated-counter.stories.tsx(1 hunks)packages/propel/src/animated-counter/animated-counter.tsx(1 hunks)packages/propel/src/animated-counter/index.ts(1 hunks)packages/propel/src/emoji-reaction/emoji-reaction-picker.stories.tsx(1 hunks)packages/propel/src/emoji-reaction/emoji-reaction-picker.tsx(1 hunks)packages/propel/src/emoji-reaction/emoji-reaction.stories.tsx(1 hunks)packages/propel/src/emoji-reaction/emoji-reaction.tsx(1 hunks)packages/propel/src/emoji-reaction/index.ts(1 hunks)packages/propel/tsdown.config.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
packages/propel/src/emoji-reaction/emoji-reaction-picker.tsx (3)
packages/propel/src/utils/placement.ts (3)
TSide(19-19)TAlign(20-20)convertPlacementToSideAndAlign(42-47)packages/propel/src/emoji-icon-picker/helper.tsx (1)
emojiToString(133-136)packages/propel/src/emoji-icon-picker/emoji/emoji.tsx (1)
EmojiRoot(10-66)
packages/propel/src/emoji-reaction/emoji-reaction-picker.stories.tsx (1)
packages/propel/src/emoji-icon-picker/helper.tsx (1)
stringToEmoji(143-154)
packages/propel/src/emoji-reaction/emoji-reaction.tsx (1)
packages/propel/src/emoji-icon-picker/helper.tsx (1)
stringToEmoji(143-154)
⏰ 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 (3)
packages/propel/src/animated-counter/animated-counter.tsx (1)
60-66: Verify that all referenced keyframe animations (slideOut,slideOutDown,slideInFromBottom,slideInFromTop,fadeOut) are defined inpackages/propel/src/styles(e.g.animations.css) and included in the build output viatsdown.config.ts.packages/propel/src/animated-counter/index.ts (1)
1-2: LGTM: clean re-exports.packages/propel/src/emoji-reaction/index.ts (1)
1-10: Re-exports are clean and complete.Values and corresponding types are exposed as expected. No issues.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
packages/propel/tsdown.config.ts (1)
19-19: Confirm source exists for emoji-reaction-picker (or drop the entry).
Previous review flagged packages/propel/src/emoji-reaction-picker/index.ts as missing; if still absent, tsdown build will fail when resolving this entry. Keep config and exports in sync.Run to verify and see import sites:
#!/bin/bash set -euo pipefail # 1) Check for the missing entry source test -f packages/propel/src/emoji-reaction-picker/index.ts && echo "OK: src entry present" || echo "MISSING: packages/propel/src/emoji-reaction-picker/index.ts" # 2) Grep for imports of the subpath (to gauge usage) rg -nP '@plane/propel/(emoji-reaction-picker|emoji-reaction)' -C2 || trueIf you intend to keep the dedicated subpath, add the thin re-export (preferred to avoid duplication):
packages/propel/src/emoji-reaction-picker/index.ts
export * from "../emoji-reaction/emoji-reaction-picker";Alternatively, drop the extra entry (and keep just ./emoji-reaction) with this diff:
- "src/emoji-reaction-picker/index.ts",packages/propel/package.json (1)
33-33: Keep ./emoji-reaction-picker export in lockstep with sources and tsdown.
If packages/propel/src/emoji-reaction-picker/index.ts is still missing, this export will point to a non-built dist target and break consumers.Quick check and fix options:
#!/bin/bash set -euo pipefail # 1) Verify source and intended dist target test -f packages/propel/src/emoji-reaction-picker/index.ts && echo "OK: source present" || echo "MISSING: packages/propel/src/emoji-reaction-picker/index.ts" # 2) Sanity-check exports keys exist for both new subpaths jq -r '.exports | keys[]' packages/propel/package.json | rg -n 'emoji-reaction($|/)|emoji-reaction-picker($|/)'If you decide not to ship a separate subpath, remove it here as well:
- "./emoji-reaction-picker": "./dist/emoji-reaction-picker/index.js",
🧹 Nitpick comments (2)
packages/propel/src/command/command.tsx (2)
22-24: Forward refs and make data-slot non-overridableCurrent signature drops ref forwarding, and
{...props}afterdata-slotallows consumers to override it. UseforwardRefand placedata-slotlast.-function CommandList({ ...props }: React.ComponentProps<typeof CommandPrimitive.List>) { - return <CommandPrimitive.List data-slot="command-list" {...props} />; -} +const CommandList = React.forwardRef< + React.ElementRef<typeof CommandPrimitive.List>, + React.ComponentPropsWithoutRef<typeof CommandPrimitive.List> +>((props, ref) => ( + <CommandPrimitive.List ref={ref} {...props} data-slot="command-list" /> +));Outside this range, add:
CommandList.displayName = "CommandList";
30-32: Same here: forward ref and lock the data-slotMirror the List changes for Item to preserve ref usage and ensure
data-slotisn’t overridden.-function CommandItem({ ...props }: React.ComponentProps<typeof CommandPrimitive.Item>) { - return <CommandPrimitive.Item data-slot="command-item" {...props} />; -} +const CommandItem = React.forwardRef< + React.ElementRef<typeof CommandPrimitive.Item>, + React.ComponentPropsWithoutRef<typeof CommandPrimitive.Item> +>((props, ref) => ( + <CommandPrimitive.Item ref={ref} {...props} data-slot="command-item" /> +));Outside this range, add:
CommandItem.displayName = "CommandItem";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/i18n/src/locales/ru/translations.json(1 hunks)packages/propel/package.json(1 hunks)packages/propel/src/command/command.tsx(1 hunks)packages/propel/src/emoji-reaction/emoji-reaction.stories.tsx(1 hunks)packages/propel/tsdown.config.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/i18n/src/locales/ru/translations.json
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/propel/src/emoji-reaction/emoji-reaction.stories.tsx
⏰ 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: Build and lint web apps
- GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
packages/propel/tsdown.config.ts (1)
18-18: LGTM: new entry matches package export and naming.
Entry for src/emoji-reaction/index.ts looks consistent with existing pattern.packages/propel/package.json (1)
32-32: LGTM: export for emoji-reaction is aligned with tsdown entry.
Consistent with existing subpath style.
* dev: animated counter added to propel * chore: animated counter story added * chore: propel config updated * chore: code refactor * dev: emoji reaction and renderer component added to propel * dev: emoji reaction story added * chore: propel config updated * chore: code refactor * fix: format error * chore: lint error resolved --------- Co-authored-by: sriram veeraghanta <veeraghanta.sriram@gmail.com>
Description
This PR introduces the emoji reaction component in Propel.
Media
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores