-
Notifications
You must be signed in to change notification settings - Fork 0
feat: better accessibility #110
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
…ted state management
…als tables using useEffect
…mic page visibility
…al count for apps, datasets, and workerpools
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…ockchainComputing/explorer-v2 into feature/add-assets-order
…and SchemaSearch components
… in Footer, Navbar, and AddressChip
…k Explorer link in SmartLinkGroup; add role to Tabs component
…and attributes to input and error messages
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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 implements accessibility improvements across the application with the following key changes:
- Adds new "ACCESS" tabs to workerpool, dataset, and app detail pages showing access/orderbook information
- Implements ARIA labels, roles, and descriptions for better screen reader support
- Refactors loading and outdated state management to bubble up from child components
- Adds semantic HTML attributes (type="search", role="searchbox", aria-live regions)
- Updates the iexec package from 8.18.0 to 8.20.0
Reviewed changes
Copilot reviewed 39 out of 40 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/routes/$chainSlug/_layout/workerpool/$workerpoolAddress.tsx | Added ACCESS tab with WorkerpoolAccessTable component and state management |
| src/routes/$chainSlug/_layout/dataset/$datasetAddress.tsx | Added ACCESS tab with DatasetAccessTable component and state management |
| src/routes/$chainSlug/_layout/app/$appAddress.tsx | Added ACCESS tab with AppAccessTable component and state management |
| src/routes/$chainSlug/_layout/address/$addressAddress.tsx | Added ACCESS tab showing all three access tables |
| src/modules/workerpools/workerpool/WorkerpoolDealsTable.tsx | Refactored to use props for loading/outdated state, fixed typo in error message |
| src/modules/workerpools/workerpool/WorkerpoolAccessTable.tsx | New component for displaying workerpool access orderbook |
| src/modules/datasets/dataset/DatasetDealsTable.tsx | Refactored to use props for loading/outdated state, fixed typo in error message |
| src/modules/datasets/dataset/DatasetAccessTable.tsx | New component for displaying dataset access orderbook |
| src/modules/apps/app/AppDealsTable.tsx | Refactored to use props for loading/outdated state, fixed typo in error message |
| src/modules/apps/app/AppAccessTable.tsx | New component for displaying app access orderbook |
| src/modules/search/SearcherBar.tsx | Added ARIA attributes for search input and error messages |
| src/modules/datasets/SchemaSearch.tsx | Added ARIA labels to filter buttons |
| src/modules/Tabs.tsx | Added role="radio" to tab buttons |
| src/components/navbar/NavBar.tsx | Added aria-label to links and buttons, changed logo alt text |
| src/components/Footer.tsx | Added aria-label to social media links |
| src/components/PaginatedNavigation.tsx | Improved pagination logic and stability |
| src/components/ModeToggle.tsx | Enhanced aria-label for theme toggle |
| src/components/CopyButton.tsx | Added aria-label and type attribute |
| package.json | Updated iexec from 8.18.0 to 8.20.0 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
PierreJeanjacquot
left a comment
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.
Minor adjustment needed on id, see comments, feel free to merge once resolved
| key={label} | ||
| data-tab-index={index} | ||
| variant="link" | ||
| role="tab" |
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.
Missing aria-selected attribute on tab elements
Medium Severity
When adding role="tab" to elements, the aria-selected attribute is required by WAI-ARIA to indicate which tab is currently active. The component tracks selection state via currentTab and displays it visually with CSS, but this information isn't communicated to assistive technologies. Screen reader users will hear these announced as tabs but won't know which one is selected, causing incorrect accessibility behavior.
| {(localError || error) && ( | ||
| <p className="bg-danger text-danger-foreground border-danger-border absolute -bottom-8 rounded-full border px-4"> | ||
| <p | ||
| id="search-error" |
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.
Hardcoded ID causes duplicate IDs across instances
Medium Severity
The id="search-error" is hardcoded, which will create duplicate IDs if SearcherBar is rendered multiple times on the same page. This violates HTML standards (IDs must be unique per document) and can cause accessibility issues where aria-describedby references the wrong error message. The PR author noted this issue in the discussion: "leads to duplicated id in the same page, we must fix this."
| <button onClick={() => onRemoveFilter(index)}> | ||
| <button | ||
| type="button" | ||
| id={`remove-filter-${index}`} |
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.
Index-based IDs risk duplicates across component instances
Low Severity
The id={remove-filter-${index}} uses an index that resets for each component instance, causing duplicate IDs if SchemaSearch is mounted multiple times on a page. Since aria-label is already set, this ID appears unnecessary and introduces an accessibility violation. The PR author flagged this: "id may be useless as aria-label is set, risk of duplicated id if the component is mounted more than once in a page."
Note
Improves accessibility semantics and screen-reader support across core UI.
aria-labels and propertype/roleattributes to interactive elements inNavbar,Footer(social links),CopyButton,AddressChip,SmartLinkGroup(external link), and logout/menu controls; logo links now havearia-label="Home"and images use emptyaltfor decorative iconsSearcherBar:type="search",role="searchbox", error messaging wired viaaria-describedby,role="alert",aria-live, and a mobile SR-only action button with contextualaria-labelsModeTogglebuttons get clearer labels;Tabsnow exposesrole="tablist"and per-tabrole="tab"SchemaSearchgains an accessible toggle and per-filter remove buttons with explicit labels/ids; buttons usetype="button"where appropriateWritten by Cursor Bugbot for commit 7bcdeb6. This will update automatically on new commits. Configure here.