fix: add missing @keyframes shake animation referenced in popup.ts#857
fix: add missing @keyframes shake animation referenced in popup.ts#857Siddh2024 wants to merge 1 commit into
Conversation
🚀 Thank You for Contributing to Late-MeetPlease ensure that:
Thank you for contributing 💙 |
|
👋 Thank you @Siddh2024 for your contribution to Late-Meet! ✅ Verified: You are assigned to the linked issue #529. Please review any automated suggestions or code review comments that may appear below! We will review your PR as soon as possible! Please consider starring the repository ⭐ to show your support! |
📝 WalkthroughWalkthroughAdds a ChangesShake animation fix
Estimated code review effort: 1 (Trivial) | ~3 minutes Sequence Diagram(s)Not applicable — changes are limited to CSS class definitions and a small class-toggling adjustment without multi-component interaction flow. Related Issues: Suggested labels: bug, css, frontend Suggested reviewers: shouri123 🐰 A shake was promised, none appeared, 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/popup.ts (1)
537-543: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAnimation won't restart on rapid repeated calls.
If
shakeElementis invoked again while"shake"is already applied (e.g. two quick invalid submissions within 400ms),classList.addis a no-op since the class is already present, so the CSS animation does not restart. A DOM reflow is needed between removing and re-adding the class for the animation to reliably retrigger.🔧 Proposed fix to force animation restart
function shakeElement(el: HTMLElement | null) { if (!el) return; - el.classList.add("shake", "border-danger"); + el.classList.remove("shake"); + void el.offsetWidth; // force reflow to restart animation + el.classList.add("shake", "border-danger"); setTimeout(() => { el.classList.remove("shake", "border-danger"); }, 400); }🤖 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 `@src/popup.ts` around lines 537 - 543, The shake animation in shakeElement can’t reliably restart on rapid repeated calls because the "shake" class stays applied, so re-adding it is a no-op. Update shakeElement to force a restart by removing the animation classes, triggering a DOM reflow on the same HTMLElement, and then re-adding "shake" and "border-danger"; keep the fix localized to shakeElement so repeated invalid submissions retrigger the animation consistently.
🤖 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 `@src/popup.ts`:
- Around line 537-543: The shake animation in shakeElement can’t reliably
restart on rapid repeated calls because the "shake" class stays applied, so
re-adding it is a no-op. Update shakeElement to force a restart by removing the
animation classes, triggering a DOM reflow on the same HTMLElement, and then
re-adding "shake" and "border-danger"; keep the fix localized to shakeElement so
repeated invalid submissions retrigger the animation consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 57857528-7630-4314-8d60-2668397b01ba
📒 Files selected for processing (2)
src/popup.csssrc/popup.ts



fixes #529
Adds the missing @Keyframes shake rule in popup.css and refactors shakeElement() to use CSS classes.
Summary by CodeRabbit