Skip to content

Conversation

@jjmata
Copy link
Collaborator

@jjmata jjmata commented Jan 15, 2026

Summary

Introduces standardized enter/exit animation patterns to the design system, building on PR #650.

  • New animation-utils.css with reusable animation presets
  • Refactored element-removal controller using Stimulus classes API
  • Updated notification partials to use design system utilities
  • Documentation added to CLAUDE.md

Test plan

  • Verify notifications fade in on page load
  • Verify notifications fade out on dismiss click
  • Test alert and CTA notifications
  • Test in both light and dark mode

Summary by CodeRabbit

  • New Features

    • Introduced smooth enter/exit animations for notifications and alerts with fade-up, fade, and scale effects.
    • Added dismiss (close) button to call-to-action notifications with smooth animated removal transitions.
    • Implemented configurable animation duration and state management for all notification elements.
  • Documentation

    • Added Animation Patterns documentation describing available animation utilities and usage examples.

✏️ Tip: You can customize this high-level summary in your review settings.

Introduce standardized enter/exit animation patterns to the design system:

- Create animation-utils.css with reusable animation presets:
  - animate-fade-up-* for notifications/toasts
  - animate-fade-* for overlays/dialogs
  - animate-scale-* for modals/popovers
  - transition-enter-exit preset for standard timing

- Refactor element-removal controller to use Stimulus classes API:
  - Supports configurable initial/visible/exit class states
  - Includes idempotent removal guard
  - Backwards compatible (works without animation classes)

- Update notification partials to use design system utilities
- Document animation patterns in CLAUDE.md
@dosubot
Copy link

dosubot bot commented Jan 15, 2026

Related Documentation

Checked 1 published document(s) in 1 knowledge base(s). No updates required.

How did I do? Any feedback?  Join Discord

@coderabbitai
Copy link

coderabbitai bot commented Jan 15, 2026

📝 Walkthrough

Walkthrough

This change introduces a new animation pattern system for the design system. It adds Tailwind CSS animation utilities for fade-up, fade, and scale transitions with initial, visible, and exit states. A new Stimulus controller manages element removal with animation lifecycle handling. Notification partials are updated to utilize these animations.

Changes

Cohort / File(s) Summary
Documentation
CLAUDE.md
Adds "Animation Patterns" section describing animation utilities, Stimulus controller integration, available presets (animate-fade-up-*, animate-fade-*, animate-scale-* with states), and usage examples. Note: Section appears duplicated in the file.
Animation Utilities
app/assets/tailwind/maybe-design-system.css, app/assets/tailwind/maybe-design-system/animation-utils.css
Imports new animation-utils module. Defines Tailwind @utility presets for enter/exit animations with fade-up, fade, and scale variants across initial, visible, and exit states, plus transition-enter-exit utility.
Stimulus Controller
app/javascript/controllers/element_removal_controller.js
Introduces element-removal controller with configurable duration (default 300ms). On connect, swaps initial class to visible via requestAnimationFrame. On remove, transitions visible to exit class, waits for animation, then removes DOM element. Includes fallback for missing animation classes.
Notification Templates
app/views/shared/notifications/_alert.html.erb, app/views/shared/notifications/_cta.html.erb, app/views/shared/notifications/_notice.html.erb
Updates notification partials to wire element-removal controller with animation lifecycle classes (initial, visible, exit). Adds animation classes (transition-enter-exit, animate-fade-up-*) and data attributes for class binding. CTA template adds new dismiss icon with click action.

Sequence Diagram(s)

sequenceDiagram
    participant DOM as DOM Element
    participant Controller as element_removal_controller
    participant Browser as Browser/Animation Engine
    participant CSS as CSS Animations

    DOM->>Controller: connect() triggered
    Controller->>Browser: requestAnimationFrame
    Browser->>DOM: Apply visible class (remove initial)
    DOM->>CSS: Trigger animation-fade-up-visible
    CSS->>Browser: Animation plays

    DOM->>Controller: click->remove() triggered
    Controller->>DOM: Apply exit class (remove visible)
    DOM->>CSS: Trigger animation-fade-up-exit
    CSS->>Browser: Exit animation plays
    Browser->>Controller: Wait for duration (300ms)
    Controller->>DOM: Remove element from DOM
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Poem

🐰✨ Animation blooms so bright,
Fade-ups dance into the light!
Elements exit with a graceful sway,
Controllers choreograph the way!
Design patterns hop-hoppity-high! 🎭

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding animation design system utilities for notifications, which aligns with the primary file changes (animation-utils.css, updated controller, and notification partial updates).
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@app/views/shared/notifications/_cta.html.erb`:
- Around line 26-28: Replace the icon-only, hover-hidden div with a proper
<button> wrapper containing the icon "x" so it gains keyboard semantics, add an
aria-label like aria-label="Dismiss notification", keep the data action data: {
action: "click->element-removal#remove" } on the button, swap the visibility
classes from hidden group-hover:inline-block to opacity-0
group-hover:opacity-100 focus:opacity-100 to ensure keyboard focus reveals it,
and replace the border-alpha-black-50 token with a functional token such as
border-secondary in the class list.
🧹 Nitpick comments (1)
app/javascript/controllers/element_removal_controller.js (1)

56-58: Consider using transitionend event instead of setTimeout.

The current implementation assumes the CSS transition duration matches durationValue. If they drift apart, the element may be removed before the animation completes or there may be unnecessary delay. Using transitionend would be more robust:

♻️ Suggested alternative
     if (this.hasVisibleClass && this.hasExitClass) {
       this.element.classList.remove(...this.visibleClasses);
       this.element.classList.add(...this.exitClasses);

-      setTimeout(() => {
-        this.element.remove();
-      }, this.durationValue);
+      this.element.addEventListener(
+        "transitionend",
+        () => this.element.remove(),
+        { once: true }
+      );
     } else {
       this.element.remove();
     }

The current approach works fine when duration is kept in sync with CSS. This is a minor improvement suggestion.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5d6abf7 and 58362ae.

📒 Files selected for processing (7)
  • CLAUDE.md
  • app/assets/tailwind/maybe-design-system.css
  • app/assets/tailwind/maybe-design-system/animation-utils.css
  • app/javascript/controllers/element_removal_controller.js
  • app/views/shared/notifications/_alert.html.erb
  • app/views/shared/notifications/_cta.html.erb
  • app/views/shared/notifications/_notice.html.erb
🧰 Additional context used
📓 Path-based instructions (27)
{app/**/*.{erb,html},app/javascript/**/*.js}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Prefer Hotwire-first approach: use Turbo + Stimulus for reactive UI without heavy JavaScript

Files:

  • app/javascript/controllers/element_removal_controller.js
  • app/views/shared/notifications/_cta.html.erb
  • app/views/shared/notifications/_notice.html.erb
  • app/views/shared/notifications/_alert.html.erb
app/javascript/controllers/**/*_controller.js

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

app/javascript/controllers/**/*_controller.js: Keep Stimulus controllers lightweight and simple with fewer than 7 targets
Keep Stimulus controllers small with private methods and clear public API

Files:

  • app/javascript/controllers/element_removal_controller.js
app/javascript/**/*.js

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use D3.js for financial visualizations (time series, donut, sankey charts)

Files:

  • app/javascript/controllers/element_removal_controller.js
**/*.{rb,js,erb}

📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)

Format currencies, numbers, dates, and other values server-side, then pass to Stimulus controllers for display only

Files:

  • app/javascript/controllers/element_removal_controller.js
  • app/views/shared/notifications/_cta.html.erb
  • app/views/shared/notifications/_notice.html.erb
  • app/views/shared/notifications/_alert.html.erb
**/*_controller.{js,ts}

📄 CodeRabbit inference engine (.cursor/rules/stimulus_conventions.mdc)

**/*_controller.{js,ts}: Use declarative actions in Stimulus controllers, not imperative event listeners - define actions in ERB templates using data-action attributes instead of binding events in the controller's connect() method
Keep Stimulus controllers lightweight and simple - aim for fewer than 7 controller targets as a sign of excessive complexity
Use private methods in Stimulus controllers and expose a clear public API
Keep Stimulus controllers focused on single or highly related responsibilities - domain logic should not belong in Stimulus controllers

Files:

  • app/javascript/controllers/element_removal_controller.js
{app/views/**,app/helpers/**,app/javascript/controllers/**}

📄 CodeRabbit inference engine (.cursor/rules/ui-ux-design-guidelines.mdc)

{app/views/**,app/helpers/**,app/javascript/controllers/**}: Reference maybe-design-system.css for base primitives, functional tokens, and component tokens before writing styles
Prefer using functional design system tokens (e.g., text-primary, bg-container, border-primary) from maybe-design-system.css instead of raw Tailwind color values

Files:

  • app/javascript/controllers/element_removal_controller.js
  • app/views/shared/notifications/_cta.html.erb
  • app/views/shared/notifications/_notice.html.erb
  • app/views/shared/notifications/_alert.html.erb
app/javascript/controllers/**/*.js

📄 CodeRabbit inference engine (.cursor/rules/view_conventions.mdc)

Use Stimulus targets to reference DOM elements, not manual getElementById calls

Files:

  • app/javascript/controllers/element_removal_controller.js
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{js,jsx,ts,tsx}: Use lowerCamelCase for variable and function names in JavaScript
Use PascalCase for class and component names in JavaScript
Let Biome format JavaScript and TypeScript code automatically

Files:

  • app/javascript/controllers/element_removal_controller.js
app/javascript/**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Place JavaScript code in app/javascript/ directory

Files:

  • app/javascript/controllers/element_removal_controller.js
**/*.{js,jsx,ts,tsx,css}

📄 CodeRabbit inference engine (AGENTS.md)

Run npm run lint and npm run format using Biome for JavaScript, TypeScript, and CSS code

Files:

  • app/javascript/controllers/element_removal_controller.js
  • app/assets/tailwind/maybe-design-system.css
  • app/assets/tailwind/maybe-design-system/animation-utils.css
**/*.{erb,js,ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{erb,js,ts,tsx}: Use query params for state management over localStorage/sessions in views
In TailwindCSS, always reference app/assets/tailwind/maybe-design-system.css for primitives and tokens
Use functional tokens defined in design system (e.g., text-primary instead of text-white, bg-container instead of bg-white)
Pass data to Stimulus controllers via data-*-value attributes, not inline JavaScript

Files:

  • app/javascript/controllers/element_removal_controller.js
  • app/views/shared/notifications/_cta.html.erb
  • app/views/shared/notifications/_notice.html.erb
  • app/views/shared/notifications/_alert.html.erb
**/*.{js,ts,tsx,erb}

📄 CodeRabbit inference engine (CLAUDE.md)

In Stimulus controllers, use Declarative Actions (HTML declares what happens). Example: data-action="click->toggle#toggle"

Files:

  • app/javascript/controllers/element_removal_controller.js
  • app/views/shared/notifications/_cta.html.erb
  • app/views/shared/notifications/_notice.html.erb
  • app/views/shared/notifications/_alert.html.erb
app/javascript/controllers/**/*.{js,ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Keep Stimulus controllers lightweight and simple (< 7 targets). Use private methods and expose clear public API.

Files:

  • app/javascript/controllers/element_removal_controller.js
app/views/**/_*.html.erb

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use partials only for simple, static HTML with minimal logic in specific contexts

app/views/**/_*.html.erb: Use underscore prefix for partial files (e.g., _trend_change.html.erb, _form_errors.html.erb, _sync_indicator.html.erb)
Place context-specific partials in relevant controller view directory (e.g., accounts/_account_sidebar_tabs.html.erb)

Files:

  • app/views/shared/notifications/_cta.html.erb
  • app/views/shared/notifications/_notice.html.erb
  • app/views/shared/notifications/_alert.html.erb
app/views/**/*.erb

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

app/views/**/*.erb: Always use the icon helper for icons in views, never use lucide_icon directly
Use Tailwind design tokens (e.g., text-primary, bg-container, border-secondary) instead of raw color names
Pass data to Stimulus controllers via data-*-value attributes instead of inline JavaScript
Prefer semantic HTML elements (dialog, details, summary) over JavaScript components
Use Turbo Frames for page sections instead of client-side solutions
Use server-side formatting for currencies, numbers, and dates instead of client-side

app/views/**/*.erb: Avoid heavy logic in ERB views; prefer helpers and components instead
Ensure ERB views pass erb-lint checks as configured in .erb_lint.yml

Files:

  • app/views/shared/notifications/_cta.html.erb
  • app/views/shared/notifications/_notice.html.erb
  • app/views/shared/notifications/_alert.html.erb
**/*.{css,erb}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use Tailwind CSS v4.x with custom design system for styling

Files:

  • app/views/shared/notifications/_cta.html.erb
  • app/assets/tailwind/maybe-design-system.css
  • app/views/shared/notifications/_notice.html.erb
  • app/views/shared/notifications/_alert.html.erb
  • app/assets/tailwind/maybe-design-system/animation-utils.css
**/*.{rb,erb}

📄 CodeRabbit inference engine (.cursor/rules/general-rules.mdc)

**/*.{rb,erb}: Use Current.user for accessing the current user. Do NOT use current_user
Use Current.family for accessing the current family. Do NOT use current_family

Files:

  • app/views/shared/notifications/_cta.html.erb
  • app/views/shared/notifications/_notice.html.erb
  • app/views/shared/notifications/_alert.html.erb
**/*.html.erb

📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)

**/*.html.erb: Prefer native HTML elements over JavaScript-based components. Use semantic HTML: for modals,

for disclosures
Leverage Turbo frames to break up pages instead of JavaScript-driven client-side solutions
Use the icon helper from application_helper.rb for icons. Never use lucide_icon helper directly

Files:

  • app/views/shared/notifications/_cta.html.erb
  • app/views/shared/notifications/_notice.html.erb
  • app/views/shared/notifications/_alert.html.erb
**/*.{rb,html.erb}

📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)

Use ActiveRecord validations for complex validations and business logic. Simple validations may be mirrored in ActiveRecord for form error handling convenience, but prioritize client-side form validation when possible

Files:

  • app/views/shared/notifications/_cta.html.erb
  • app/views/shared/notifications/_notice.html.erb
  • app/views/shared/notifications/_alert.html.erb
{app/views/**,app/helpers/**}

📄 CodeRabbit inference engine (.cursor/rules/ui-ux-design-guidelines.mdc)

Always generate semantic HTML

Files:

  • app/views/shared/notifications/_cta.html.erb
  • app/views/shared/notifications/_notice.html.erb
  • app/views/shared/notifications/_alert.html.erb
**/*.{erb,html.erb}

📄 CodeRabbit inference engine (.cursor/rules/view_conventions.mdc)

**/*.{erb,html.erb}: Use ViewComponents when: element has complex logic or styling patterns, will be reused across multiple views/contexts, needs structured styling with variants/sizes, requires interactive behavior or Stimulus controllers, has configurable slots or complex APIs, or needs accessibility features or ARIA support
Use Partials when: element is primarily static HTML with minimal logic, used in only one or few specific contexts, is simple template content, doesn't need variants/sizes/complex configuration, or is more about content organization than reusable functionality
Prefer components over partials: if a component is available in app/components, use it; if not, look for a partial; if neither exists, decide between component or partial based on complexity and reusability criteria
Keep domain logic out of views: compute values like button classes, conditional logic, and data transformations in the component file, not the template file
Always use the declarative approach when integrating Stimulus controllers in views: the ERB template should declare what happens using data-* attributes, and the Stimulus controller should respond
Component controllers in app/components/ should only be used within their component templates; global controllers in app/javascript/controllers/ can be used across any view
Pass data from Rails to Stimulus using data-*-value attributes, not inline JavaScript

Files:

  • app/views/shared/notifications/_cta.html.erb
  • app/views/shared/notifications/_notice.html.erb
  • app/views/shared/notifications/_alert.html.erb
**/*.erb

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.erb: In view templates, use native HTML elements preferred over JS components: use <dialog> for modals, <details><summary> for disclosures
Leverage Turbo frames for page sections over client-side solutions in views
Always use server-side formatting for currencies, numbers, and dates in views
Always use icon helper in application_helper.rb. NEVER use lucide_icon directly in templates.
Always generate semantic HTML in view templates
Keep domain logic OUT of view templates. Logic belongs in component files, not template files.

Files:

  • app/views/shared/notifications/_cta.html.erb
  • app/views/shared/notifications/_notice.html.erb
  • app/views/shared/notifications/_alert.html.erb
**/*.{erb,rb}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{erb,rb}: All user-facing strings must use localization (i18n). Always use t() helper. Update locale files for each new or changed element.
Use i18n interpolation for dynamic content: t("users.greeting", name: user.name)
Use Rails pluralization in i18n: t("transactions.count", count: @transactions.count)

Files:

  • app/views/shared/notifications/_cta.html.erb
  • app/views/shared/notifications/_notice.html.erb
  • app/views/shared/notifications/_alert.html.erb
app/assets/tailwind/maybe-design-system.css

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Never create new styles in design system files without permission

NEVER create new styles in design system files without permission

Files:

  • app/assets/tailwind/maybe-design-system.css
app/assets/**/*.css

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Avoid adding new global styles to design system files without permission

Files:

  • app/assets/tailwind/maybe-design-system.css
  • app/assets/tailwind/maybe-design-system/animation-utils.css
app/assets/tailwind/{maybe-design-system,application}.css

📄 CodeRabbit inference engine (.cursor/rules/ui-ux-design-guidelines.mdc)

Never create new styles in maybe-design-system.css or application.css without explicit permission

Files:

  • app/assets/tailwind/maybe-design-system.css
app/assets/**

📄 CodeRabbit inference engine (AGENTS.md)

Place styles (Tailwind), images, and fonts in app/assets/ directory

Files:

  • app/assets/tailwind/maybe-design-system.css
  • app/assets/tailwind/maybe-design-system/animation-utils.css
🧠 Learnings (31)
📓 Common learnings
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:54:59.198Z
Learning: Applies to **/*.{css,erb} : Use Tailwind CSS v4.x with custom design system for styling
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/ui-ux-design-guidelines.mdc:0-0
Timestamp: 2025-11-24T16:57:09.166Z
Learning: Applies to {app/views/**,app/helpers/**,app/javascript/controllers/**} : Prefer using functional design system tokens (e.g., text-primary, bg-container, border-primary) from maybe-design-system.css instead of raw Tailwind color values
📚 Learning: 2025-11-24T16:54:59.198Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:54:59.198Z
Learning: Applies to app/javascript/controllers/**/*_controller.js : Keep Stimulus controllers small with private methods and clear public API

Applied to files:

  • app/javascript/controllers/element_removal_controller.js
📚 Learning: 2026-01-10T19:11:44.566Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T19:11:44.566Z
Learning: Applies to app/javascript/controllers/**/*.{js,ts,tsx} : Keep Stimulus controllers lightweight and simple (< 7 targets). Use private methods and expose clear public API.

Applied to files:

  • app/javascript/controllers/element_removal_controller.js
📚 Learning: 2025-11-24T16:56:51.982Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/stimulus_conventions.mdc:0-0
Timestamp: 2025-11-24T16:56:51.982Z
Learning: Applies to **/*_controller.{js,ts} : Use declarative actions in Stimulus controllers, not imperative event listeners - define actions in ERB templates using data-action attributes instead of binding events in the controller's connect() method

Applied to files:

  • app/javascript/controllers/element_removal_controller.js
  • app/views/shared/notifications/_cta.html.erb
📚 Learning: 2025-11-24T16:56:51.982Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/stimulus_conventions.mdc:0-0
Timestamp: 2025-11-24T16:56:51.982Z
Learning: Applies to **/*_controller.{js,ts} : Keep Stimulus controllers lightweight and simple - aim for fewer than 7 controller targets as a sign of excessive complexity

Applied to files:

  • app/javascript/controllers/element_removal_controller.js
📚 Learning: 2025-09-23T22:22:00.149Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/stimulus_conventions.mdc:0-0
Timestamp: 2025-09-23T22:22:00.149Z
Learning: Applies to **/*_controller.{js,ts} : Keep Stimulus controllers lightweight: aim for fewer than 7 targets and expose a small public API with other logic in private methods

Applied to files:

  • app/javascript/controllers/element_removal_controller.js
📚 Learning: 2025-11-24T16:56:51.982Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/stimulus_conventions.mdc:0-0
Timestamp: 2025-11-24T16:56:51.982Z
Learning: Applies to **/*_controller.{js,ts} : Use private methods in Stimulus controllers and expose a clear public API

Applied to files:

  • app/javascript/controllers/element_removal_controller.js
📚 Learning: 2025-09-23T22:23:22.963Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/view_conventions.mdc:0-0
Timestamp: 2025-09-23T22:23:22.963Z
Learning: Applies to app/{views,components}/**/*.html.erb : Integrate Stimulus declaratively in ERB: templates declare data-controller/actions/targets; controllers respond to those declarations

Applied to files:

  • app/javascript/controllers/element_removal_controller.js
  • app/views/shared/notifications/_notice.html.erb
  • app/views/shared/notifications/_alert.html.erb
📚 Learning: 2025-09-23T22:22:00.149Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/stimulus_conventions.mdc:0-0
Timestamp: 2025-09-23T22:22:00.149Z
Learning: Applies to **/*_controller.{js,ts} : Keep Stimulus controllers focused on UI behavior; avoid domain/business logic and prefer single, closely related responsibilities; use Stimulus callbacks, actions, targets, values, and classes

Applied to files:

  • app/javascript/controllers/element_removal_controller.js
📚 Learning: 2025-11-24T16:54:59.198Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:54:59.198Z
Learning: Applies to app/javascript/controllers/**/*_controller.js : Keep Stimulus controllers lightweight and simple with fewer than 7 targets

Applied to files:

  • app/javascript/controllers/element_removal_controller.js
📚 Learning: 2026-01-10T19:11:44.566Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T19:11:44.566Z
Learning: Applies to **/*.{js,ts,tsx,erb} : In Stimulus controllers, use Declarative Actions (HTML declares what happens). Example: `data-action="click->toggle#toggle"`

Applied to files:

  • app/javascript/controllers/element_removal_controller.js
📚 Learning: 2025-11-24T16:56:13.406Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/project-conventions.mdc:0-0
Timestamp: 2025-11-24T16:56:13.406Z
Learning: Applies to **/*.html.erb : Prefer native HTML elements over JavaScript-based components. Use semantic HTML: <dialog> for modals, <details><summary> for disclosures

Applied to files:

  • app/views/shared/notifications/_cta.html.erb
  • app/views/shared/notifications/_notice.html.erb
  • app/views/shared/notifications/_alert.html.erb
📚 Learning: 2026-01-10T19:11:44.566Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T19:11:44.566Z
Learning: Applies to **/*.erb : In view templates, use native HTML elements preferred over JS components: use `<dialog>` for modals, `<details><summary>` for disclosures

Applied to files:

  • app/views/shared/notifications/_cta.html.erb
  • app/views/shared/notifications/_notice.html.erb
  • app/views/shared/notifications/_alert.html.erb
  • CLAUDE.md
📚 Learning: 2025-11-24T16:54:59.198Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:54:59.198Z
Learning: Applies to app/views/**/*.erb : Prefer semantic HTML elements (dialog, details, summary) over JavaScript components

Applied to files:

  • app/views/shared/notifications/_cta.html.erb
  • app/views/shared/notifications/_notice.html.erb
  • app/views/shared/notifications/_alert.html.erb
📚 Learning: 2025-11-24T16:54:59.198Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:54:59.198Z
Learning: Always reference app/assets/tailwind/maybe-design-system.css for design system tokens and primitives

Applied to files:

  • app/assets/tailwind/maybe-design-system.css
  • app/assets/tailwind/maybe-design-system/animation-utils.css
📚 Learning: 2025-11-24T16:54:59.198Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:54:59.198Z
Learning: Applies to app/assets/tailwind/maybe-design-system.css : Never create new styles in design system files without permission

Applied to files:

  • app/assets/tailwind/maybe-design-system.css
  • app/assets/tailwind/maybe-design-system/animation-utils.css
📚 Learning: 2026-01-10T19:11:44.566Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T19:11:44.566Z
Learning: Applies to **/*.{erb,js,ts,tsx} : In TailwindCSS, always reference `app/assets/tailwind/maybe-design-system.css` for primitives and tokens

Applied to files:

  • app/assets/tailwind/maybe-design-system.css
  • app/assets/tailwind/maybe-design-system/animation-utils.css
📚 Learning: 2025-11-24T16:57:09.166Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/ui-ux-design-guidelines.mdc:0-0
Timestamp: 2025-11-24T16:57:09.166Z
Learning: Applies to app/assets/tailwind/{maybe-design-system,application}.css : Never create new styles in maybe-design-system.css or application.css without explicit permission

Applied to files:

  • app/assets/tailwind/maybe-design-system.css
  • app/assets/tailwind/maybe-design-system/animation-utils.css
📚 Learning: 2025-11-24T16:57:09.166Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/ui-ux-design-guidelines.mdc:0-0
Timestamp: 2025-11-24T16:57:09.166Z
Learning: Applies to {app/views/**,app/helpers/**,app/javascript/controllers/**} : Prefer using functional design system tokens (e.g., text-primary, bg-container, border-primary) from maybe-design-system.css instead of raw Tailwind color values

Applied to files:

  • app/assets/tailwind/maybe-design-system.css
  • app/assets/tailwind/maybe-design-system/animation-utils.css
  • CLAUDE.md
📚 Learning: 2025-11-24T16:54:59.198Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:54:59.198Z
Learning: Applies to app/assets/**/*.css : Avoid adding new global styles to design system files without permission

Applied to files:

  • app/assets/tailwind/maybe-design-system.css
📚 Learning: 2025-11-24T16:54:59.198Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:54:59.198Z
Learning: Applies to **/*.{css,erb} : Use Tailwind CSS v4.x with custom design system for styling

Applied to files:

  • app/assets/tailwind/maybe-design-system.css
  • app/assets/tailwind/maybe-design-system/animation-utils.css
📚 Learning: 2026-01-10T19:11:44.566Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-10T19:11:44.566Z
Learning: Applies to **/*.{erb,js,ts,tsx} : Use functional tokens defined in design system (e.g., `text-primary` instead of `text-white`, `bg-container` instead of `bg-white`)

Applied to files:

  • app/assets/tailwind/maybe-design-system.css
  • CLAUDE.md
📚 Learning: 2025-11-24T16:57:09.166Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/ui-ux-design-guidelines.mdc:0-0
Timestamp: 2025-11-24T16:57:09.166Z
Learning: Applies to {app/views/**,app/helpers/**,app/javascript/controllers/**} : Reference maybe-design-system.css for base primitives, functional tokens, and component tokens before writing styles

Applied to files:

  • app/assets/tailwind/maybe-design-system.css
  • CLAUDE.md
📚 Learning: 2025-11-24T16:57:20.318Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/view_conventions.mdc:0-0
Timestamp: 2025-11-24T16:57:20.318Z
Learning: Applies to **/*.{erb,html.erb} : Always use the declarative approach when integrating Stimulus controllers in views: the ERB template should declare what happens using data-* attributes, and the Stimulus controller should respond

Applied to files:

  • app/views/shared/notifications/_notice.html.erb
  • app/views/shared/notifications/_alert.html.erb
📚 Learning: 2025-09-23T22:23:22.963Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/view_conventions.mdc:0-0
Timestamp: 2025-09-23T22:23:22.963Z
Learning: Applies to app/views/**/*.html.erb : Use ViewComponents when elements have complex logic/styling, are reused across contexts, need variants/slots/accessibility, or require Stimulus interactivity

Applied to files:

  • app/views/shared/notifications/_notice.html.erb
  • app/views/shared/notifications/_alert.html.erb
  • CLAUDE.md
📚 Learning: 2025-11-24T16:57:20.318Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/view_conventions.mdc:0-0
Timestamp: 2025-11-24T16:57:20.318Z
Learning: Applies to **/*.{erb,html.erb} : Use ViewComponents when: element has complex logic or styling patterns, will be reused across multiple views/contexts, needs structured styling with variants/sizes, requires interactive behavior or Stimulus controllers, has configurable slots or complex APIs, or needs accessibility features or ARIA support

Applied to files:

  • app/views/shared/notifications/_notice.html.erb
  • app/views/shared/notifications/_alert.html.erb
  • CLAUDE.md
📚 Learning: 2025-09-23T22:22:00.149Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/stimulus_conventions.mdc:0-0
Timestamp: 2025-09-23T22:22:00.149Z
Learning: Applies to app/{views,components}/**/*.html.erb : In ERB views, wire Stimulus interactions declaratively using data-action to call controller methods

Applied to files:

  • app/views/shared/notifications/_alert.html.erb
📚 Learning: 2025-11-24T16:54:59.198Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:54:59.198Z
Learning: Applies to app/components/**/*.rb : Use ViewComponents for reusable or complex UI elements instead of partials

Applied to files:

  • CLAUDE.md
📚 Learning: 2025-11-24T16:54:59.198Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:54:59.198Z
Learning: Applies to app/components/**/*.rb : Use viewComponents and Stimulus controllers organized alongside components in app/components/

Applied to files:

  • CLAUDE.md
📚 Learning: 2025-11-24T16:57:20.318Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/view_conventions.mdc:0-0
Timestamp: 2025-11-24T16:57:20.318Z
Learning: Applies to **/*.{erb,html.erb} : Keep domain logic out of views: compute values like button classes, conditional logic, and data transformations in the component file, not the template file

Applied to files:

  • CLAUDE.md
📚 Learning: 2025-09-23T22:23:22.963Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/view_conventions.mdc:0-0
Timestamp: 2025-09-23T22:23:22.963Z
Learning: Applies to app/views/**/*.html.erb : Prefer components over partials: use an available component from app/components; if none, consider a partial; otherwise decide using the criteria

Applied to files:

  • CLAUDE.md
⏰ 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: ci / test
🔇 Additional comments (8)
app/assets/tailwind/maybe-design-system/animation-utils.css (1)

1-58: LGTM! Well-structured animation utilities.

The three-state animation pattern (initial/visible/exit) is clean and the documentation header clearly explains usage. The @utility directive correctly uses Tailwind CSS v4.x syntax.

Note: Per coding guidelines, new design system styles typically require explicit permission. The PR description indicates this addition is intentional.

app/assets/tailwind/maybe-design-system.css (1)

13-13: LGTM!

The import follows the established pattern for design system utility modules.

app/javascript/controllers/element_removal_controller.js (2)

31-35: LGTM! Clean Stimulus controller with proper classes API usage.

The controller is lightweight with zero targets and a clear public API (remove()). The classes API usage (initial, visible, exit) is idiomatic Stimulus.


37-46: LGTM! Proper use of requestAnimationFrame for enter animation.

The RAF ensures the browser has painted the initial state before transitioning to visible, which is necessary for CSS transitions to animate properly.

app/views/shared/notifications/_notice.html.erb (1)

3-10: LGTM! Proper declarative Stimulus integration with animation lifecycle.

The data attributes correctly configure the element-removal controller's classes API. The animationend action triggers from the inner SVG progress animation (line 31), while the enter/exit animations control visibility—these work independently as intended.

app/views/shared/notifications/_alert.html.erb (1)

3-9: LGTM! Clean alert notification with proper animation setup.

The alert correctly uses manual dismissal (click action only, no auto-dismiss). The animation classes are properly configured for the element-removal controller.

app/views/shared/notifications/_cta.html.erb (1)

4-10: Animation controller integration looks good.

The Stimulus controller data attributes are correctly structured with the classes API pattern, and the animation utilities align with the new design system.

CLAUDE.md (1)

222-251: Documentation is well-structured and consistent with implementation.

The Animation Patterns section provides clear guidance on available presets, states, and usage with the element-removal controller. The example matches the implementation in the notification partials, and the important note about avoiding hardcoded animation classes is valuable guidance.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

Comment on lines +26 to +28
<div class="absolute -top-2 -right-2">
<%= icon "x", class: "p-0.5 hidden group-hover:inline-block border border-alpha-black-50 border-solid rounded-lg bg-container text-subdued cursor-pointer", data: { action: "click->element-removal#remove" } %>
</div>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Accessibility concern: close button needs keyboard visibility and accessible label.

Two issues with the dismiss button:

  1. Missing accessible label: The icon-only button lacks an aria-label for screen reader users.
  2. Hidden until hover: Using hidden group-hover:inline-block makes the button invisible to keyboard-only users who cannot trigger hover states.

Also, border-alpha-black-50 may not be a functional design system token — prefer border-secondary or similar if available.

♿ Suggested fix for accessibility
     <div class="absolute -top-2 -right-2">
-      <%= icon "x", class: "p-0.5 hidden group-hover:inline-block border border-alpha-black-50 border-solid rounded-lg bg-container text-subdued cursor-pointer", data: { action: "click->element-removal#remove" } %>
+      <button type="button"
+              aria-label="<%= t('notifications.dismiss') %>"
+              class="p-0.5 opacity-0 group-hover:opacity-100 focus:opacity-100 border border-secondary border-solid rounded-lg bg-container text-subdued cursor-pointer transition-opacity"
+              data-action="click->element-removal#remove">
+        <%= icon "x", size: "xs" %>
+      </button>
     </div>

Using opacity-0/100 with focus:opacity-100 ensures keyboard users can reveal the button when focused. Wrapping in a <button> element provides proper semantics. Based on learnings, prefer functional tokens like border-secondary over raw values.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<div class="absolute -top-2 -right-2">
<%= icon "x", class: "p-0.5 hidden group-hover:inline-block border border-alpha-black-50 border-solid rounded-lg bg-container text-subdued cursor-pointer", data: { action: "click->element-removal#remove" } %>
</div>
<div class="absolute -top-2 -right-2">
<button type="button"
aria-label="<%= t('notifications.dismiss') %>"
class="p-0.5 opacity-0 group-hover:opacity-100 focus:opacity-100 border border-secondary border-solid rounded-lg bg-container text-subdued cursor-pointer transition-opacity"
data-action="click->element-removal#remove">
<%= icon "x", size: "xs" %>
</button>
</div>
🤖 Prompt for AI Agents
In `@app/views/shared/notifications/_cta.html.erb` around lines 26 - 28, Replace
the icon-only, hover-hidden div with a proper <button> wrapper containing the
icon "x" so it gains keyboard semantics, add an aria-label like
aria-label="Dismiss notification", keep the data action data: { action:
"click->element-removal#remove" } on the button, swap the visibility classes
from hidden group-hover:inline-block to opacity-0 group-hover:opacity-100
focus:opacity-100 to ensure keyboard focus reveals it, and replace the
border-alpha-black-50 token with a functional token such as border-secondary in
the class list.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants