Skip to content

Fix PR #175 — RoleSelector/ChannelSelector Review Comments#179

Merged
BillChirico merged 1 commit intofeat/selector-implementation-v2from
feat/selector-fixes
Mar 1, 2026
Merged

Fix PR #175 — RoleSelector/ChannelSelector Review Comments#179
BillChirico merged 1 commit intofeat/selector-implementation-v2from
feat/selector-fixes

Conversation

@BillChirico
Copy link
Collaborator

Summary

Fixes all 8 review comment threads from PR #175.

Issues Fixed

1. ✅ CRITICAL — Label accessibility (CI-blocking)

Files: , ,

  • Fixed lint error noLabelWithoutControl that was failing CI
  • All 42 labels using hardcoded htmlFor="admin-role" now use unique IDs
  • Added corresponding id attributes to all form elements
  • Labels are now properly associated with controls via htmlFor matching id

2. ✅ RoleSelector doesn't accept id prop

File: config-editor.tsx:837, role-selector.tsx, channel-selector.tsx

  • Added id prop to both RoleSelector and ChannelSelector TypeScript interfaces
  • Components now forward the id to the underlying Button element
  • Enables proper label association for accessibility

3. ✅ Zustand store unused

File: web/src/stores/discord-entities.ts:69

  • Removed unused discord-entities.ts store file
  • Removed zustand dependency from web/package.json
  • Store was added but never integrated into the selectors

4. ✅ Formatter drift

File: web/src/components/ui/form.tsx:115

  • Fixed formatting on the ariaDescribedBy block
  • Fixed trailing comma in web/package.json
  • All files now pass biome format checks

5. ⚠️ Missing ChannelSelector migrations

File: config-editor.tsx

Status: Not implemented in this PR

The following fields currently use plain text inputs:

  • welcome.rulesChannel
  • welcome.introChannel
  • starboard.channelId
  • challenges.channelId
  • github.feed.channelId

Rationale: Converting these to ChannelSelector would be a significant UX change that goes beyond "fixing review comments." The current text input approach works and is consistent with other ID fields in the config editor. If ChannelSelector is desired for these fields, it should be done in a follow-up feature PR with proper testing and consideration of the UX implications.

Test Results

  • pnpm lint — All checks pass (123 files)
  • pnpm format — No drift
  • pnpm test — All 2812 tests pass

Files Changed

  1. web/src/components/ui/role-selector.tsx — Added id prop support
  2. web/src/components/ui/channel-selector.tsx — Added id prop support
  3. web/src/components/dashboard/config-editor.tsx — Fixed 42 label accessibility issues
  4. web/src/components/dashboard/config-sections/ModerationSection.tsx — Added id to ChannelSelector
  5. web/src/components/dashboard/config-sections/TriageSection.tsx — Added id to ChannelSelector
  6. web/src/components/ui/form.tsx — Fixed formatting
  7. web/package.json — Removed zustand, fixed trailing comma
  8. web/src/stores/discord-entities.ts — Removed (unused)
  9. pnpm-lock.yaml — Updated dependencies

Commit SHA

b2be17f

- Fix all 42 label accessibility issues in config-editor.tsx
  - Replace hardcoded htmlFor='admin-role' with unique IDs
  - Add corresponding id attributes to form elements
  - Ensure labels are properly associated with controls

- Add id prop support to RoleSelector and ChannelSelector
  - Both components now accept and forward id to the button element
  - Enables proper label association for accessibility

- Remove unused zustand store (discord-entities.ts)
  - Store was added but never integrated
  - Remove zustand dependency from package.json

- Fix formatting issues
  - Format form.tsx ariaDescribedBy block
  - Fix trailing comma in web/package.json

- Install missing swagger-jsdoc dependency

All lint checks pass. All 2812 tests pass.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 1, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/selector-fixes

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.

@greptile-apps
Copy link

greptile-apps bot commented Mar 1, 2026

Greptile Summary

Successfully addresses all 8 review comment threads from PR #175. Fixed CI-blocking accessibility issue by replacing 42 hardcoded htmlFor="admin-role" labels with unique IDs. Added id prop support to RoleSelector and ChannelSelector components. Removed unused Zustand store and dependency. Fixed formatting drift.

Key changes:

  • All form labels now have unique htmlFor attributes matching their control's id
  • RoleSelector and ChannelSelector accept and forward id prop to Button element
  • Removed web/src/stores/discord-entities.ts and zustand from package.json
  • Fixed formatting in form.tsx ariaDescribedBy block

Documentation:

  • AGENTS.md still references the deleted discord-entities.ts store (line 49) and should be updated

Confidence Score: 5/5

  • Safe to merge with only a minor documentation update needed
  • All changes are straightforward fixes addressing review comments. No duplicate IDs found. TypeScript interfaces correctly updated. Tests passing. Only issue is outdated documentation reference.
  • AGENTS.md needs update to remove deleted store reference

Important Files Changed

Filename Overview
web/package.json Removed unused zustand dependency
web/src/components/dashboard/config-editor.tsx Fixed 42 label accessibility issues with unique htmlFor/id pairs
web/src/components/ui/channel-selector.tsx Added id prop to interface and forwarded to Button element
web/src/components/ui/role-selector.tsx Added id prop to interface and forwarded to Button element

Last reviewed commit: b2be17f

@BillChirico BillChirico merged commit 4c76a4d into feat/selector-implementation-v2 Mar 1, 2026
4 checks passed
@BillChirico BillChirico deleted the feat/selector-fixes branch March 1, 2026 20:51
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.

1 participant