fix: copyable Tag animation state #586
Conversation
🦋 Changeset detectedLatest commit: f4a8878 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Summary by CodeRabbit
WalkthroughThe changes update the copy animation behavior for the copyable Tag component in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TagComponent
participant ClipboardAPI
User->>TagComponent: Clicks copyable tag
TagComponent->>ClipboardAPI: Calls writeText(label)
ClipboardAPI-->>TagComponent: Confirms text copied
TagComponent->>TagComponent: Sets showCopiedAnimation = true
Note over TagComponent: Animation plays
TagComponent->>TagComponent: After 600ms, sets showCopiedAnimation = false
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes were found. ✨ Finishing Touches
🧪 Generate Unit Tests
🪧 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 (
|
cheshtagupta284
force-pushed
the
MI-779-fix-tag
branch
from
8e7e0b5 to
e5c55c4
Compare
cheshtagupta284
force-pushed
the
MI-779-fix-tag
branch
from
850c660 to
f4a8878
Compare
![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 (3)
.changeset/silver-kiwis-attend.md (1)
5-5: Consider a more descriptive changeset message.While the current message is clear, consider providing more specific details about the fix for better changelog documentation.
-fixes animation for copyable tag component +fix copyable Tag animation persistence causing text overflowpackages/react/src/components/Tag/tests/Tag.test.tsx (1)
24-46: Comprehensive test for copy functionality.The test properly mocks the clipboard API and verifies the core copy behavior. Good use of the label prop for clipboard content verification.
Consider adding a test to verify the animation timing behavior:
it('should show and hide animation state correctly', async () => { jest.useFakeTimers(); const mockClipboard = { writeText: jest.fn().mockResolvedValue(undefined) }; Object.defineProperty(global.navigator, 'clipboard', { value: mockClipboard, writable: true, }); render(<Tag isCopyable label="test">Test</Tag>); const tag = screen.getByText('Test').closest('.manifest-tag'); fireEvent.click(tag!); // Animation should be active initially expect(tag).toHaveClass('showCopiedAnimation'); // Fast-forward past the timeout jest.advanceTimersByTime(700); // Animation should be cleared expect(tag).not.toHaveClass('showCopiedAnimation'); jest.useRealTimers(); });packages/react/src/components/Tag/Tag.tsx (1)
121-132: Solid animation timing implementation with proper cleanup.The useEffect correctly manages the animation lifecycle - triggering on copy and clearing after 600ms. The cleanup function prevents memory leaks.
Consider extracting the timeout duration as a constant for better maintainability:
+const ANIMATION_DURATION = 600; // slightly less than 0.7s CSS animation React.useEffect(() => { let timeoutId: ReturnType<typeof setTimeout> | undefined; if (isCopied) { setShowCopiedAnimation(true); - timeoutId = setTimeout(() => void setShowCopiedAnimation(false), 600); // slightly less than animation duration + timeoutId = setTimeout(() => void setShowCopiedAnimation(false), ANIMATION_DURATION); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.changeset/silver-kiwis-attend.md(1 hunks)packages/react/src/components/Tag/Tag.styles.ts(1 hunks)packages/react/src/components/Tag/Tag.tsx(2 hunks)packages/react/src/components/Tag/story/Tag.stories.tsx(1 hunks)packages/react/src/components/Tag/tests/Tag.test.tsx(1 hunks)
🔇 Additional comments (5)
packages/react/src/components/Tag/Tag.styles.ts (1)
68-68: LGTM: Variant name correctly reflects its purpose.The rename from
isCopiedtoshowCopiedAnimationbetter separates the animation state from the copy state, which addresses the core issue of the persistent ::before content.packages/react/src/components/Tag/story/Tag.stories.tsx (2)
76-76: Excellent accessibility improvements.The updated aria-labels correctly reflect the copyable functionality instead of removal, significantly improving accessibility for screen reader users.
Also applies to: 84-84, 92-92
83-86: Good addition for testing the overflow fix.The "Tag with long text" example effectively demonstrates the scenario where the animation overflow issue would occur, making it easier to verify the fix visually.
packages/react/src/components/Tag/Tag.tsx (2)
67-67: Good separation of animation state.The new
showCopiedAnimationstate properly separates animation control from the copy state, addressing the core issue of persistent animation content.
113-113: Correct integration with styling system.The
showCopiedAnimationstate is properly passed to the useStyles hook, enabling the CSS variant to control the animation display.
Closes #
📝 Description
Affected component:
Copyable variant of Tag
Issue:
The content inside ::before persisted after copy icon was clicked. For tag with long text, this ::before content overflowed the container even though the original text was ellipsed.
RCA:
isCopied state wrongly controlled animation state.
Fix:
Separated the animation state which triggers on first copy only.
Screenshots
After fix:
Screen.Recording.2025-06-24.at.5.21.24.PM.mov
Before fix:
Screen.Recording.2025-06-25.at.9.16.47.AM.mov
Merge checklist