🎨 Palette: Add accessible aria-label to settings navigation link#953
🎨 Palette: Add accessible aria-label to settings navigation link#953
Conversation
Co-authored-by: anchapin <6326294+anchapin@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdds an accessible aria-label to the settings navigation link and updates the top navigation links config and rendering logic to support optional aria labels for links. Flow diagram for TopNavigation link configuration with optional ariaLabelflowchart TD
A[TopNavigation_component] --> B[Define_navigationLinks_array]
B --> C[Link_home_to_/]
B --> D[Link_experiments_to_/experiments]
B --> E[Link_results_to_/experiment-results]
B --> F[Link_docs_to_/docs]
B --> G[Link_settings_to_/settings_with_label_span_aria-hidden_true_and_ariaLabel_Settings]
H[Render_navigation_links] --> I[Iterate_over_navigationLinks]
I --> J[For_each_link_create_Link_component]
J --> K[Set_Link_to_property_from_link_to]
J --> L[Set_Link_content_from_link_label]
J --> M[Set_optional_aria-label_from_link_ariaLabel]
A --> H
B --> H
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Consider updating the type definition for
navigationLinks(and any related props/interfaces) so thatlabelcan be aReactNodeandariaLabelis explicitly typed as an optional string, which will make this pattern clearer and prevent implicitanyusage.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider updating the type definition for `navigationLinks` (and any related props/interfaces) so that `label` can be a `ReactNode` and `ariaLabel` is explicitly typed as an optional string, which will make this pattern clearer and prevent implicit `any` usage.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull request overview
This PR improves the accessibility of the Top Navigation settings link by ensuring screen readers announce a meaningful name (“Settings”) instead of reading the gear emoji.
Changes:
- Replaced the settings link’s emoji-only label with an emoji wrapped in
aria-hidden="true". - Added an explicit
aria-label="Settings"for the settings navigation link while leaving other links unchanged.
Co-authored-by: anchapin <6326294+anchapin@users.noreply.github.com>
Co-authored-by: anchapin <6326294+anchapin@users.noreply.github.com>
<span aria-hidden="true">.PR created automatically by Jules for task 320742299427355609 started by @anchapin
Summary by Sourcery
Improve accessibility of the top navigation settings link by providing a descriptive ARIA label and hiding the decorative emoji from assistive technologies.
Enhancements: