HOLD: Audit Stimulus controllers for conventions (split into smaller pr's) by maebeale · Pull Request #1392 · rubyforgood/awbw

maebeale · 2026-03-10T12:51:44Z

Summary

Audit all Stimulus controllers to follow project conventions
Use Stimulus targets instead of querySelector / direct DOM queries
Use hidden class instead of inline style.display manipulation
Rename JS constants from SCREAMING_SNAKE_CASE to camelCase
Use shorthand action descriptors (omit default events)
Remove unused controllers (nav, search_box)
Remove debug console.log statements
Add Stimulus conventions to CLAUDE.md based on PR review feedback

Test plan

Column toggle (events manage)

Toggle columns on/off in event registrations table
Knob slides correctly (Tailwind class instead of inline transform)

Comment edit toggle

Event registration form — toggle inline comment editing on/off
Organization form — toggle inline comment editing on/off
Person form — toggle inline comment editing on/off
User form — toggle inline comment editing on/off
Nested field values sync correctly between view and edit modes

Dirty form (unsaved changes warning)

Person form — warns before navigating away with unsaved changes
User form — warns before navigating away with unsaved changes
Submit clears dirty state (no false warning after save)

Password toggle (eye icon)

Login page — toggle password visibility
Set password page — toggle both password fields independently
Change password page — toggle all 3 password fields independently
Welcome/first-login page — toggle password visibility

Tabs controller

Tabbed interfaces switch content correctly (index-based lookup)

Paginated fields

Multi-page comment fields — navigate between pages

Tags combination highlight

Explore by combination — sector/category checkboxes highlight correctly
Highlights sync after turbo frame load

Print options

Print toggle — check/x icons swap correctly

Toggle lock controller

Lock icon shows/hides using hidden class

Toggle user icon controller

User form — icon toggles on/off
User edit — icon toggles on/off

Removed controllers

No broken references to data-controller="nav" or data-controller="search-box"

🤖 Generated with Claude Code

maebeale · 2026-03-10T14:59:06Z

@jmilljr24 how does this look?

jmilljr24

I think there is too much in this for me to feel confident about the changes. I gave a couple of examples where things just didn't make sense. There is definitely some value within these changes but there needs to be quite a bit more manual validation.

Some of the clean up (remove unused controller or case conventions) seem fine. Anything else I'd rather do in smaller PR's with some more attention and user testing. Maybe update a controller if a feature being worked on involves it.

jmilljr24 · 2026-03-10T18:50:15Z

app/frontend/javascript/controllers/collection_controller.js

-      if (type === "checkbox") {
-        this.toggleClass(event.target);
-      }
+  handleChange(event) {


By creating this function, this change essential us just duplicating what was already in the connect function but now requires added a stimulus action in the html (extra code). By using connect which is built in to stimulus, the code automatically runs once the stimulus element is connected. Thats what this.element is.

There is other additional code additions in this controller that don't seem necessary. (target.type === "search" ?)

jmilljr24 · 2026-03-10T18:51:10Z

app/frontend/javascript/controllers/column_toggle_controller.js

These seem fine as they are part of the conversation we had.

jmilljr24 · 2026-03-10T18:55:42Z

app/frontend/javascript/controllers/dropdown_controller.js

    this.processPayload(event.params.payload);
    this.manageEventListeners();
-    this.open = !this.open;
+    this.openValue = !this.openValue;


Stimulus values are used to track or update a value in the dom. this.open is simply an instance variable/property we are using to keep track of the state. We don't need an actual value in the dom, the js controller instance itself can can track it.

jmilljr24 · 2026-03-10T19:04:16Z

app/frontend/javascript/controllers/print_options_controller.js

This might be ok.

jmilljr24 · 2026-03-10T19:06:10Z

app/frontend/javascript/controllers/remote_select_controller.js

 import { Controller } from "@hotwired/stimulus";
 import TomSelect from "tom-select";

+const styleId = "remote-select-overrides";


I don't see why this is needed.

jmilljr24 · 2026-03-10T19:08:49Z

app/views/bookmarks/_search_boxes.html.erb

 <%= form_with url: url,
              method: :get,
              data: { controller: "collection",
+                      action: "change->collection#handleChange input->collection#handleInput",


Suggested change

action: "change->collection#handleChange input->collection#handleInput",

We get this for free with stimulus if we keep the logic in the connect function.

- Remove dead controllers (nav, search_box) - Convert querySelector/getElementById to static targets - Convert instance variables to static values - Replace addEventListener with data-action attributes - Replace style.display with hidden class toggling - Fix CSS injection leak in remote_select - Remove console.log statements - Add comprehensive Stimulus conventions to AI instruction files - Add AI Instruction Files reference table to CLAUDE.md - Update PR instruction wording to "content" (covers checklists) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

- tag_deduping.rake no longer exists, remove from AGENTS.md - Add instruction to update AI files on every push when conventions, architecture, or file lists change Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

- copilot-instructions.md now just points to CLAUDE.md and AGENTS.md - AGENTS.md removes duplicated PR, Git, and frontend preference sections - CLAUDE.md updated to reflect the new structure Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

CLAUDE.md should be lean — only coding rules and conventions that need to be in every conversation context. Project overview, tech stack, setup, key directories, testing details, linting/security commands all moved to AGENTS.md where agents can look them up on demand. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot inline suggestions can't follow file references, so it needs its own copy of code style, RuboCop, Stimulus, and HTML/ERB rules. Without these, Copilot suggestions regularly fail RuboCop CI. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

- Rake tasks: 17 → 4 (removed nonexistent paperclip_to_active_storage) - Services: 15 → 21 (added event_registration, user, and other services) - Stimulus controllers: 20 listed → all 34 listed - Model concerns: added AuthorCreditable - Controller concerns: added TagAssignable - Updated all directory and spec counts to match actual codebase - Jobs: 2 → 3 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

List the exact things to check: Stimulus controllers, services, concerns, mailers, rake tasks, and file counts. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Add convention to AI files and rename existing constants: STYLE_ID → styleId, ACTIVE_CLASSES → activeClasses, GRAY_CLASSES → grayClasses, HIGHLIGHT_CLASS → highlightClass, FILE_TYPE_ICONS → fileTypeIcons Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Prefer Font Awesome free icons over inline SVGs by default. Inline SVGs still allowed when a specific icon design is preferred. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Keep the full name in the project overview, use "this codebase" elsewhere to avoid hardcoding the name throughout. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

- events_spec: update data-column-toggle-col to data-column-toggle-target="col" - password_field_toggle_spec: scope toggle click within controller wrapper and use eventually matcher for async type change Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

The collection controller was refactored to use data-action instead of addEventListener, but only video_recordings was updated. All 17 other forms using the collection controller were missing the action attributes, breaking search/filter auto-submit. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

- collection_controller: Keep addEventListener in connect() — idiomatic when listening on the controller's root element, avoids data-action attributes in 18 views - dropdown_controller: Use this.open instance property instead of Stimulus value — values are for DOM-reflected state, not internal tracking - remote_select_controller: Remove styleId guard — unnecessary abstraction - paginated_fields_controller: Use this.currentPage instance property - comment_edit_toggle_controller: Use this.editing instance property Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

- addEventListener in connect() is fine for root-element events - Use instance properties for internal state, not static values - Don't add guards without evidence of a bug Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

jmilljr24 reviewed Mar 10, 2026

View reviewed changes

maebeale and others added 19 commits March 10, 2026 19:09

Remove stale tag_deduping.rake reference, add AI files update rule

049eb0e

- tag_deduping.rake no longer exists, remove from AGENTS.md - Add instruction to update AI files on every push when conventions, architecture, or file lists change Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Add draft PR workflow — push early, mark ready when done

5da8e14

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Expand AI files trigger phrases in CLAUDE.md

59a6e7d

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Add mailer to Related Files checklist in CLAUDE.md

72dd7dd

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Make AGENTS.md update instruction more specific

db0445e

List the exact things to check: Stimulus controllers, services, concerns, mailers, rake tasks, and file counts. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Add Font Awesome icon preference to AI files

9158025

Prefer Font Awesome free icons over inline SVGs by default. Inline SVGs still allowed when a specific icon design is preferred. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Replace project name with generic references in AGENTS.md

37464b3

Keep the full name in the project overview, use "this codebase" elsewhere to avoid hardcoding the name throughout. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Remove hardcoded email from mailer description in AGENTS.md

ca437c5

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Prompt AI to suggest AI file updates for repeated corrections

4a1cd24

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Remove AI file changes — moved to separate PR (#1390)

dc9f60f

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

maebeale force-pushed the maebeale/1391-stimulus-audit branch from acd06a5 to a5cad9a Compare March 10, 2026 23:09

Add Stimulus conventions from PR review feedback

254cd70

- addEventListener in connect() is fine for root-element events - Use instance properties for internal state, not static values - Don't add guards without evidence of a bug Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

maebeale changed the title ~~Audit Stimulus controllers for conventions~~ HOLD: Audit Stimulus controllers for conventions (split into smaller pr's) Mar 11, 2026

maebeale marked this pull request as draft March 11, 2026 12:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HOLD: Audit Stimulus controllers for conventions (split into smaller pr's)#1392

HOLD: Audit Stimulus controllers for conventions (split into smaller pr's)#1392
maebeale wants to merge 20 commits intomainfrom
maebeale/1391-stimulus-audit

maebeale commented Mar 10, 2026 •

edited

Loading

Uh oh!

maebeale commented Mar 10, 2026

Uh oh!

jmilljr24 left a comment

Uh oh!

jmilljr24 Mar 10, 2026

Uh oh!

jmilljr24 Mar 10, 2026 •

edited

Loading

Uh oh!

jmilljr24 Mar 10, 2026

Uh oh!

jmilljr24 Mar 10, 2026

Uh oh!

jmilljr24 Mar 10, 2026

Uh oh!

jmilljr24 Mar 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	action: "change->collection#handleChange input->collection#handleInput",

Conversation

maebeale commented Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Column toggle (events manage)

Comment edit toggle

Dirty form (unsaved changes warning)

Password toggle (eye icon)

Tabs controller

Paginated fields

Tags combination highlight

Print options

Toggle lock controller

Toggle user icon controller

Removed controllers

Uh oh!

maebeale commented Mar 10, 2026

Uh oh!

jmilljr24 left a comment

Choose a reason for hiding this comment

Uh oh!

jmilljr24 Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

jmilljr24 Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmilljr24 Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

jmilljr24 Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

jmilljr24 Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

jmilljr24 Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

maebeale commented Mar 10, 2026 •

edited

Loading

jmilljr24 Mar 10, 2026 •

edited

Loading