Improve accessibility with semantic buttons, ARIA labels, and color contrast updates#593
Improve accessibility with semantic buttons, ARIA labels, and color contrast updates#593
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 34 minutes and 58 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThis PR introduces a CSS custom property for primary colors, converts non-semantic HTML elements to buttons with ARIA labels for accessibility, and updates styling across multiple components. The color system migrates from hard-coded hex values to a centralized Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
akita
|
||||||||||||||||||||||||||||
| Project |
akita
|
| Branch Review |
accessibility
|
| Run status |
|
| Run duration | 01m 47s |
| Commit |
|
| Committer | Joseph Rhoads |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
|
|
0
|
|
|
0
|
|
|
3
|
|
|
0
|
|
|
47
|
| View all changes introduced in this branch ↗︎ | |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/SearchBox/SearchBox.tsx (1)
51-68:⚠️ Potential issue | 🟠 MajorClear action is still non-semantic and not keyboard-accessible.
The submit button labeling is good, but the clear control remains a clickable
<span>. Use a real button so it is focusable and keyboard operable.🔧 Suggested fix
{searchInput !== '' && ( - <span + <Button id="search-clear-facets" + type="button" title="Clear" - aria-label="Clear" + aria-label="Clear search input" + className="p-0 border-0 bg-transparent" onClick={onSearchClear} > <FontAwesomeIcon icon={faTimes} /> - </span> + </Button> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/SearchBox/SearchBox.tsx` around lines 51 - 68, The clear control is implemented as a non-semantic, non-keyboard-accessible <span> (id "search-clear-facets") inside SearchBox; replace it with a proper button element so it is focusable and keyboard operable, keep the existing title/aria-label ("Clear"), move the onClick handler to the button (use the existing onSearchClear callback), and preserve the FontAwesomeIcon and conditional rendering (searchInput !== '') and any className/style to maintain appearance and behavior.
🧹 Nitpick comments (2)
src/styles.css (1)
691-694: Include:focus-visiblewith hover styles for keyboard parity.This keeps visual feedback consistent for keyboard users, not just mouse hover.
♿ Suggested refinement
-.button:hover { +.button:hover, +.button:focus-visible { background-color: `#0071b2`; color: `#fff`; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/styles.css` around lines 691 - 694, The :hover rule for .button only targets mouse users; add a matching :focus-visible selector to provide equivalent keyboard focus styling. Update the CSS where .button:hover is defined so that .button:focus-visible receives the same background-color and color, and ensure any outline or box-shadow for focus is preserved or set explicitly to maintain accessibility; reference the .button:hover selector and the .button:focus-visible state when applying the change.src/components/DataCiteButton/DataCiteButton.tsx (1)
17-20: Consider using the global CSS variable as the single source of truth.
#243b54is now duplicated in TS and global CSS. Referencing the existing CSS variable here reduces drift risk.♻️ Suggested refactor
const COLORS = { primary: '#037AAD', - secondary: '#243b54', + secondary: 'var(--datacite-color-primary, `#243b54`)', }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/DataCiteButton/DataCiteButton.tsx` around lines 17 - 20, The COLORS constant in DataCiteButton.tsx duplicates the palette from global CSS (COLORS { primary, secondary } contains '#243b54'); replace the hardcoded secondary color with the global CSS custom property instead (use the CSS var in the component styles or set COLORS.secondary = 'var(--your-global-secondary-var)') so the component reads the single source of truth; update any usage of COLORS.secondary in DataCiteButton to use that CSS variable name (or remove COLORS entirely and reference the CSS var directly in the component's style/class).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/FilterItem/FilterItem.tsx`:
- Around line 43-50: This button is a toggle so add an aria-pressed boolean to
announce its state; update the button in FilterItem to include aria-pressed={/*
boolean active state */} by using the existing state checker (e.g., an
isActive(id) or by deriving a truthy value from activeIcon(id) or the
component's active prop) and keep the onClick calling toggleFilter(id); ensure
the attribute uses the same id/name context (name, id, activeIcon, toggleFilter)
so screen readers receive true/false for the pressed state.
In `@src/components/Header/Search.tsx`:
- Around line 60-63: The clear button currently calls setSearchInput('') and
search('') but the search function reads the module state searchInput (not the
query argument), causing an empty ?query= to be serialized; update the search
function (search) to prefer the passed-in query parameter (use the function
parameter if defined, falling back to searchInput) and when the resolved query
is an empty string or null remove the query param from the URL instead of
serializing ?query=; ensure the onClick handler (which calls setSearchInput('')
and search('')) continues to work with this change.
---
Outside diff comments:
In `@src/components/SearchBox/SearchBox.tsx`:
- Around line 51-68: The clear control is implemented as a non-semantic,
non-keyboard-accessible <span> (id "search-clear-facets") inside SearchBox;
replace it with a proper button element so it is focusable and keyboard
operable, keep the existing title/aria-label ("Clear"), move the onClick handler
to the button (use the existing onSearchClear callback), and preserve the
FontAwesomeIcon and conditional rendering (searchInput !== '') and any
className/style to maintain appearance and behavior.
---
Nitpick comments:
In `@src/components/DataCiteButton/DataCiteButton.tsx`:
- Around line 17-20: The COLORS constant in DataCiteButton.tsx duplicates the
palette from global CSS (COLORS { primary, secondary } contains '#243b54');
replace the hardcoded secondary color with the global CSS custom property
instead (use the CSS var in the component styles or set COLORS.secondary =
'var(--your-global-secondary-var)') so the component reads the single source of
truth; update any usage of COLORS.secondary in DataCiteButton to use that CSS
variable name (or remove COLORS entirely and reference the CSS var directly in
the component's style/class).
In `@src/styles.css`:
- Around line 691-694: The :hover rule for .button only targets mouse users; add
a matching :focus-visible selector to provide equivalent keyboard focus styling.
Update the CSS where .button:hover is defined so that .button:focus-visible
receives the same background-color and color, and ensure any outline or
box-shadow for focus is preserved or set explicitly to maintain accessibility;
reference the .button:hover selector and the .button:focus-visible state when
applying the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: abd0ee27-dfc2-47ef-b811-e61b8a0e7b3e
📒 Files selected for processing (9)
src/components/DataCiteButton/DataCiteButton.tsxsrc/components/FacetList/FacetListItem.module.scsssrc/components/FilterItem/FilterItem.tsxsrc/components/Footer/Footer.tsxsrc/components/Header/Search.tsxsrc/components/HelpIcon/HelpIcon.tsxsrc/components/SearchBox/SearchBox.tsxsrc/components/WorkMetadata/WorkMetadata.module.scsssrc/styles.css
- Use semantic button for facet search clear control (SearchBox) - Fix Header search() to use passed query and drop empty query param - Add aria-pressed to filter toggle button - Align DataCiteButton secondary color with CSS variable - Add :focus-visible parity for .button and facet clear focus ring Co-authored-by: Joseph Rhoads <jrhoads@users.noreply.github.com>
- Use semantic button for facet search clear control (SearchBox) - Fix Header search() to use passed query and drop empty query param - Add aria-pressed to filter toggle button - Align DataCiteButton secondary color with CSS variable - Add :focus-visible parity for .button and facet clear focus ring Co-authored-by: Joseph Rhoads <jrhoads@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/Header/Search.tsx`:
- Line 38: The current router.push call in Search.tsx builds
`${base}?${params.toString()}` which yields a trailing '?' when
params.toString() is empty; update the logic around router.push (the code using
base and params.toString()) to check if params.toString() is non-empty and push
`${base}?${params.toString()}` only in that case, otherwise push just base
(e.g., compute the URL conditionally before calling router.push).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: bab434dc-8d43-43a7-a5f3-6b6beda737a8
📒 Files selected for processing (7)
src/components/DataCiteButton/DataCiteButton.tsxsrc/components/FacetList/FacetListItem.module.scsssrc/components/FilterItem/FilterItem.tsxsrc/components/Header/Search.tsxsrc/components/SearchBox/SearchBox.tsxsrc/components/WorkMetadata/WorkMetadata.module.scsssrc/styles.css
✅ Files skipped from review due to trivial changes (4)
- src/components/WorkMetadata/WorkMetadata.module.scss
- src/components/FacetList/FacetListItem.module.scss
- src/components/FilterItem/FilterItem.tsx
- src/components/DataCiteButton/DataCiteButton.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- src/styles.css
- src/components/SearchBox/SearchBox.tsx
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
akita
|
||||||||||||||||||||||||||||
| Project |
akita
|
| Branch Review |
1.47.0
|
| Run status |
|
| Run duration | 01m 45s |
| Commit |
|
| Committer | Joseph Rhoads |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
|
|
0
|
|
|
0
|
|
|
3
|
|
|
0
|
|
|
47
|
| View all changes introduced in this branch ↗︎ | |
Purpose
The purpose of this pull request is to improve the accessibility (a11y) and visual consistency of the application. This includes updating color contrasts to meet standards, adding descriptive ARIA labels to interactive elements, and converting non-semantic interactive tags into semantic buttons.
Approach
Key Modifications
aria-labelto social links in the footer, search buttons, help icons, and filter items.<a>tag filter toggle inFilterItemto a semantic<button>.<span>in the Header to a semantic<Button>component.#A3ACB2to a darker#243b54for better contrast and visibility.--datacite-color-primary) for consistent theming across components likeFacetListItemandWorkMetadata.Important Technical Details
<a>or<span>to<button>to ensure correct screen reader behavior and keyboard focus.:rootvariables allows for easier theme management in the future.styles.cssto accommodate its transition to a button element.Types of changes
Reviewer, please remember our guidelines:
Summary by CodeRabbit
New Features
Accessibility Improvements
Style