-
Notifications
You must be signed in to change notification settings - Fork 175
Fix: Enable useAriaPropsSupportedByRole biome rule for @liam-hq/app package #3452
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- Enable useAriaPropsSupportedByRole rule in biome.jsonc for @liam-hq/app package - Fix ChatInput textarea ARIA attributes by removing unsupported role and attributes - Add role="option" to MentionSuggestor buttons to support aria-selected - Add role="listbox" to MentionSuggestor container for proper ARIA pattern - Maintain accessibility while ensuring biome rule compliance 🤖 Generated with [Claude Code](https://claude.ai/code) Co-authored-by: FunamaYukina <FunamaYukina@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughEnabled the Biome a11y rule useAriaPropsSupportedByRole in the app package and adjusted chat components’ ARIA usage: removed incorrect combobox-related attributes from the textarea, made aria-controls conditional to the suggestor state, and added appropriate listbox/option roles to the mention suggestor. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant ChatInput as ChatInput (textarea)
participant Mention as MentionSuggestor (popover)
User->>ChatInput: Type / mention trigger
ChatInput->>Mention: Open suggestor (state: open)
Note over ChatInput,Mention: ChatInput sets aria-controls=<mentionSuggestorId> only when open
Mention-->>User: Render options (role=listbox/option)
User->>ChatInput: Continue typing / selection
alt Suggestor closes
ChatInput->>Mention: Close suggestor (state: closed)
Note over ChatInput: aria-controls removed (undefined)
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Suggested labels
Suggested reviewers
Pre-merge checks (5 passed)✅ Passed checks (5 passed)
Poem
✨ Finishing touches
🧪 Generate unit tests
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 |
Updates to Preview Branch (claude/issue-3447-20250911-1308) ↗︎
Tasks are run on every commit but only new migration files are pushed.
View logs for this Workflow Run ↗︎. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enables the useAriaPropsSupportedByRole
biome rule for the @liam-hq/app package to improve accessibility by ensuring ARIA attributes are used correctly with appropriate HTML elements. The changes fix existing violations in chat components by adding proper roles and removing unsupported ARIA attributes.
- Enable the
useAriaPropsSupportedByRole
accessibility rule in biome configuration - Fix ARIA attribute violations in chat input component by removing unsupported attributes from textarea
- Fix mention suggestion component by adding proper
listbox
andoption
roles
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
frontend/apps/app/biome.jsonc | Enables the useAriaPropsSupportedByRole accessibility rule |
frontend/apps/app/components/SessionDetailPage/components/Chat/components/ChatInput/ChatInput.tsx | Removes unsupported ARIA attributes from textarea element |
frontend/apps/app/components/SessionDetailPage/components/Chat/components/ChatInput/components/MentionSuggestor/MentionSuggestor.tsx | Adds proper listbox and option roles to support ARIA attributes |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
aria-controls={ | ||
isMentionSuggestorOpen ? mentionSuggestorId : undefined | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The textarea element still needs proper ARIA attributes for combobox functionality. Consider wrapping the textarea in a div with role='combobox'
and the appropriate ARIA attributes, or add role='combobox'
to the textarea itself if supported by your target browsers.
Copilot uses AI. Check for mistakes.
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/apps/app/components/SessionDetailPage/components/Chat/components/ChatInput/components/MentionSuggestor/MentionSuggestor.tsx (2)
124-126
: Effect doesn’t match its comment; never resets on query changeDependency array is empty. Reset when
query
changes.- useEffect(() => { - setHighlightedIndex(0) - }, []) + useEffect(() => { + setHighlightedIndex(0) + }, [query])
128-141
: Scroll-to-active doesn’t run when selection changesThe effect depends only on
enabled
. IncludehighlightedIndex
so scrolling tracks navigation.- }, [enabled]) + }, [enabled, highlightedIndex])
♻️ Duplicate comments (1)
frontend/apps/app/components/SessionDetailPage/components/Chat/components/ChatInput/ChatInput.tsx (1)
195-197
: On prior suggestion about “combobox on textarea”Ignoring the earlier recommendation to add combobox semantics to a textarea is correct here;
textarea
isn’t a valid combobox control. Your current approach is safer.
🧹 Nitpick comments (3)
frontend/apps/app/components/SessionDetailPage/components/Chat/components/ChatInput/components/MentionSuggestor/MentionSuggestor.tsx (3)
146-146
: Name the listboxProvide an accessible name so AT can announce the popup meaningfully.
- <div ref={containerRef} className={styles.wrapper} id={id} role="listbox"> + <div ref={containerRef} className={styles.wrapper} id={id} role="listbox" aria-label="Mention suggestions">
147-151
: Avoid non-option content inside listboxThe empty-state message inside a listbox can confuse AT. Prefer rendering the message outside the listbox as a polite status.
- <div ref={containerRef} className={styles.wrapper} id={id} role="listbox"> - {matches.length === 0 ? ( - <div className={styles.emptyMessage} aria-disabled="true"> - {DEFAULT_NO_ITEMS_MESSAGE} - </div> - ) : ( - matches.map(…) - )} - </div> + {matches.length === 0 ? ( + <div className={styles.emptyMessage} role="status" aria-live="polite"> + {DEFAULT_NO_ITEMS_MESSAGE} + </div> + ) : ( + <div ref={containerRef} className={styles.wrapper} id={id} role="listbox" aria-label="Mention suggestions"> + {matches.map(…)} + </div> + )}
117-121
: Remove unnecessary event type castYou only use
e.key
. Generalize the handler type and drop the cast.- useImperativeHandle(ref, () => ({ - handleKeyDown(e) { - handleKeyDown(e as unknown as KeyboardEvent<HTMLInputElement>) - }, - })) + useImperativeHandle(ref, () => ({ + handleKeyDown(e: KeyboardEvent<HTMLElement>) { + handleKeyDown(e) + }, + }))And update the local handler signature:
- const handleKeyDown = useCallback( - (e: KeyboardEvent<HTMLInputElement>) => { + const handleKeyDown = useCallback( + (e: KeyboardEvent<HTMLElement>) => {
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
frontend/apps/app/biome.jsonc
(1 hunks)frontend/apps/app/components/SessionDetailPage/components/Chat/components/ChatInput/ChatInput.tsx
(1 hunks)frontend/apps/app/components/SessionDetailPage/components/Chat/components/ChatInput/components/MentionSuggestor/MentionSuggestor.tsx
(2 hunks)
⏰ 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). (3)
- GitHub Check: Supabase Preview
- GitHub Check: Supabase Preview
- GitHub Check: Supabase Preview
🔇 Additional comments (4)
frontend/apps/app/biome.jsonc (1)
8-11
: LGTM — linter verification failed (command missing)Enabling a11y.useAriaPropsSupportedByRole at the app package level looks good; running
pnpm lint:biome
failed ("Command "lint:biome" not found"), so I cannot confirm zero violations.Run the package linter and paste the output to verify. Suggested actions:
- Inspect frontend/apps/app/package.json for a lint script and run it.
- cd frontend/apps/app && pnpm run lint
- or from repo root: pnpm --filter ./frontend/apps/app run lint
frontend/apps/app/components/SessionDetailPage/components/Chat/components/ChatInput/components/MentionSuggestor/MentionSuggestor.tsx (1)
153-170
: Incorrect — role="option" is allowed on ; keep the button.ARIA-in-HTML's allowed roles table permits role="option" on the HTML
element (W3C/MDN). The suggested replacement with a non-button element is unnecessary; retain the button markup and preserve the pointer handlers (onMouseDown/onMouseEnter) to avoid focus loss.Likely an incorrect or invalid review comment.
frontend/apps/app/components/SessionDetailPage/components/Chat/components/ChatInput/ChatInput.tsx (2)
195-197
: Good: aria-controls only when popup is openThis removes invalid combobox bits from the textarea while preserving linkage to the popup.
195-199
: Optional a11y: advertise popup type and autocompleteAdd aria-haspopup="listbox" and aria-autocomplete="list" to the textarea (valid on multiline textbox) to improve screen‑reader hints.
data-error={error ? 'true' : undefined} + aria-haspopup="listbox" + aria-autocomplete="list" aria-controls={ isMentionSuggestorOpen ? mentionSuggestorId : undefined }
Verification: pnpm lint:biome is not available in this repo (command returned "Command 'lint:biome' not found"). Run your project's Biome linter (pnpm lint:biome or equivalent) and confirm these changes pass.
Closes #3447
Summary
Enable useAriaPropsSupportedByRole biome rule for @liam-hq/app package
Implementation Details
Overview
The
useAriaPropsSupportedByRole
biome rule needs to be enabled for the@liam-hq/app
package to improve accessibility by ensuring ARIA attributes are used correctly with appropriate HTML elements. After enabling this rule, lint errors have been detected that need to be fixed.Steps to Enable the Rule
frontend/apps/app/biome.jsonc
with the following configuration:This overrides the base configuration in
frontend/internal-packages/configs/biome.jsonc
whereuseAriaPropsSupportedByRole
is set to"off"
.Lint Errors to Fix
After enabling the rule,
useAriaPropsSupportedByRole
errors appear in the following files:components/SessionDetailPage/components/Chat/components/ChatInput/ChatInput.tsx
components/SessionDetailPage/components/Chat/components/ChatInput/components/MentionSuggestor/MentionSuggestor.tsx
These violations can lead to:
Root Cause
The chat components contain ARIA attributes that are not supported by their HTML elements:
<textarea>
element witharia-expanded
attribute (not supported by textarea)<button>
element witharia-selected
attribute (requires specific roles to be valid)Required Fix Patterns
Fix 1: Handle Textarea Expandable State
Fix 2: Fix Button Selection State
Action Items
useAriaPropsSupportedByRole: "error"
rule infrontend/apps/app/biome.jsonc
pnpm lint:biome
to identify all ARIA attribute violationscomponents/SessionDetailPage/components/Chat/components/ChatInput/ChatInput.tsx
components/SessionDetailPage/components/Chat/components/ChatInput/components/MentionSuggestor/MentionSuggestor.tsx
pnpm lint
to verify all errors are resolvedTesting
After implementing all fixes:
Related
useAriaPropsSupportedByRole
across all packages for better accessibilityAuto-generated
This PR was automatically generated by Claude Code Action.
Summary by CodeRabbit
Bug Fixes
Chores