feat(ui): apply icon-swap transition to clipboard/check icons#215
feat(ui): apply icon-swap transition to clipboard/check icons#215RedStar071 wants to merge 2 commits into
Conversation
- Add .t-icon-swap CSS block to utilities.css (transitions.dev #9) - Both icons stacked in the same grid cell; state toggles via data-state - Apply to copy-user-id button in profile.vue - Apply to copy-server-id button in guild settings General.vue - prefers-reduced-motion honored via existing global kill-switch in utilities.css
|
Warning Review limit reached
More reviews will be available in 53 minutes and 46 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (15)
📝 WalkthroughWalkthroughThis PR introduces a reusable CSS-based icon swap animation utility and applies it to improve visual feedback on two copy-to-clipboard buttons. The animation smoothly cross-fades between icons with configurable timing, blur, and scale effects. ChangesIcon Swap Animation Feature
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #215 +/- ##
==========================================
+ Coverage 66.52% 66.53% +0.01%
==========================================
Files 103 103
Lines 2306 2307 +1
Branches 472 472
==========================================
+ Hits 1534 1535 +1
Misses 412 412
Partials 360 360
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/assets/css/utilities.css (1)
384-391: 💤 Low valueConsider conditionally applying
will-changefor better performance.The
will-changeproperty is currently set permanently on all.t-iconelements within the swap container. Best practice recommends applyingwill-changeonly during the actual transition rather than statically, to avoid unnecessary compositor layers.Since there are only a few icon-swap buttons per page, the current impact is minimal, but you could optimise by moving
will-changeto a state-based rule (e.g.,.t-icon-swap:hover .t-iconor duringdata-statetransitions).♻️ Optional optimisation
.t-icon-swap .t-icon { grid-area: 1 / 1; transition: opacity var(--icon-swap-dur) var(--icon-swap-ease), filter var(--icon-swap-dur) var(--icon-swap-ease), transform var(--icon-swap-dur) var(--icon-swap-ease); - will-change: opacity, filter, transform; } + +.t-icon-swap:hover .t-icon, +.t-icon-swap:focus-within .t-icon { + will-change: opacity, filter, transform; +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/assets/css/utilities.css` around lines 384 - 391, The current .t-icon-swap .t-icon rule applies will-change permanently; remove will-change from that base rule and instead add a state-based rule (e.g., .t-icon-swap:hover .t-icon, .t-icon-swap:focus .t-icon and/or .t-icon-swap[data-state="active"] .t-icon) that sets will-change: opacity, filter, transform only while the swap is active, ensuring the existing transitions (opacity/filter/transform using --icon-swap-dur and --icon-swap-ease) still apply; this limits compositor hints to the actual transition period and improves performance.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@app/assets/css/utilities.css`:
- Around line 384-391: The current .t-icon-swap .t-icon rule applies will-change
permanently; remove will-change from that base rule and instead add a
state-based rule (e.g., .t-icon-swap:hover .t-icon, .t-icon-swap:focus .t-icon
and/or .t-icon-swap[data-state="active"] .t-icon) that sets will-change:
opacity, filter, transform only while the swap is active, ensuring the existing
transitions (opacity/filter/transform using --icon-swap-dur and
--icon-swap-ease) still apply; this limits compositor hints to the actual
transition period and improves performance.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 698345b9-dd7b-49c4-9989-a69fdd5d6bc5
📒 Files selected for processing (3)
app/assets/css/utilities.cssapp/components/guild/settings/General.vueapp/pages/profile.vue
There was a problem hiding this comment.
Pull request overview
Applies the transitions.dev #9 "icon-swap" pattern to the two clipboard ↔ check copy buttons (profile page and guild general settings), replacing the dynamic :name/:icon ternary with both icons rendered in the same grid cell and cross-faded via CSS based on data-state. Adds the reusable .t-icon-swap utility globally; relies on the existing reduced-motion kill switch.
Changes:
- Add
.t-icon-swapCSS utility and--icon-swap-*custom properties inutilities.css. - Refactor profile copy-user-id button to render both icons inside a
.t-icon-swapwrapper. - Refactor guild settings copy-server-id button similarly, moving the icon out of
:iconinto a#leadingslot.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| app/assets/css/utilities.css | New .t-icon-swap block + root custom properties for the cross-fade transition. |
| app/pages/profile.vue | Copy-user-id button now renders both clipboard/check icons inside the swap wrapper. |
| app/components/guild/settings/General.vue | Copy-server-id button switched from :icon prop to #leading slot using the swap wrapper. |
| <template #leading> | ||
| <span class="t-icon-swap" :data-state="copied ? 'b' : 'a'" aria-hidden="true"> | ||
| <UIcon name="heroicons:clipboard-document" class="t-icon" data-icon="a" /> | ||
| <UIcon name="heroicons:check" class="t-icon" data-icon="b" /> | ||
| </span> | ||
| </template> |
| <span | ||
| class="t-icon-swap" | ||
| :data-state="copied ? 'b' : 'a'" | ||
| aria-hidden="true" | ||
| > | ||
| <UIcon | ||
| name="heroicons:clipboard-document" | ||
| class="t-icon" | ||
| data-icon="a" | ||
| /> | ||
| <UIcon name="heroicons:check" class="t-icon" data-icon="b" /> | ||
| </span> |
| .t-icon-swap .t-icon { | ||
| grid-area: 1 / 1; | ||
| transition: | ||
| opacity var(--icon-swap-dur) var(--icon-swap-ease), | ||
| filter var(--icon-swap-dur) var(--icon-swap-ease), | ||
| transform var(--icon-swap-dur) var(--icon-swap-ease); | ||
| will-change: opacity, filter, transform; | ||
| } |
- Introduce avatar group hover transition with customizable lift, scale, and easing. - Implement error state shake transition for form validation feedback. - Create a comprehensive SKILL.md for transitions with usage examples and decision rules. - Add universal CSS root variables for consistent transition styling. - Update skills-lock.json to include transitions-dev skill.
🔗 Linked issue
No issue — cosmetic/UI improvement.
🧭 Context
The project uses the
transitions-devskill for production-ready CSS transitions. The icon-swap pattern (transitions.dev #9) is the correct primitive for any slot where two icons alternate based on state (e.g. clipboard → check on copy).Previously both copy buttons swapped icons via a dynamic
:nameternary on a singleUIcon, which gives an abrupt replace with no visual continuity.📚 Description
Applies the icon-swap transition (transitions.dev #9) to both clipboard ↔ check icon pairs in the app:
app/assets/css/utilities.css— adds the.t-icon-swapCSS block with:rootcustom properties (--icon-swap-dur,--icon-swap-blur,--icon-swap-start-scale,--icon-swap-ease). Both icons sit in the sameinline-gridcell;data-state="a|b"drives which icon is visible via opacity + blur + scale transition. No JavaScript required.app/pages/profile.vue— copy-user-id button: replaces the single dynamicUIconwith at-icon-swapwrapper holding both icons permanently in the DOM.app/components/guild/settings/General.vue— copy-server-id button: same pattern; icon moved into a#leadingslot sinceUButtonno longer receives:icondirectly.prefers-reduced-motionis already honored globally by the existing kill-switch inutilities.css(transition-duration: 0.01ms !important), so no per-component guard is needed.Intentionally skipped:
ColorModeButton.vue— sun/moon swap is driven by a full-page View Transition clip animation; applyingt-icon-swapinside a VT snapshot would be redundant.DisabledCommands.vuechevron — single icon rotating 180°, not a two-icon swap.