🎨 Palette: Add accessible table headers and caption to telemetry timeline#97
🎨 Palette: Add accessible table headers and caption to telemetry timeline#97igor-holt wants to merge 1 commit into
Conversation
…e table Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@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. |
There was a problem hiding this comment.
Pull request overview
This PR improves the accessibility of the telemetry timeline’s raw data table by adding proper table semantics for screen readers.
Changes:
- Added an accessible (visually hidden)
<caption>to provide context for the telemetry details table. - Added
scope="col"to<th>elements to explicitly associate column headers with their cells. - Added a short journal entry documenting the visually-hidden styling approach in the absence of a global utility class.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| components/TelemetryTimeline.tsx | Adds a visually hidden table caption and explicit column header scoping for improved screen-reader navigation. |
| .Jules/palette.md | Documents the visually-hidden pattern used due to the lack of a global .sr-only utility in this repo. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Code Review
This pull request improves the accessibility of the TelemetryTimeline component by adding a descriptive table caption and defining column scopes for headers. It also updates the project's documentation to include a pattern for visually hiding elements. Review feedback recommends replacing the deprecated clip CSS property with clipPath: 'inset(50%)' and suggests centralizing these styles into a reusable .sr-only utility class in globals.css to improve maintainability and keep the JSX clean.
| @@ -0,0 +1,3 @@ | |||
| ## 2024-11-20 - Accessible Visually Hidden Elements without Global Utilities | |||
| **Learning:** The `thrml` repository web app (`app/globals.css`) does not provide standard utility classes like `.sr-only` for visually hiding content (e.g., screen-reader captions) while keeping it accessible. | |||
| **Action:** When a screen-reader element needs to be visually hidden without breaking layout and no utility class exists, apply an explicit inline style pattern instead: `style={{ position: 'absolute', width: '1px', height: '1px', padding: 0, margin: '-1px', overflow: 'hidden', clip: 'rect(0, 0, 0, 0)', whiteSpace: 'nowrap', borderWidth: 0 }}`. This ensures accessibility guidelines are met within the constraints of the project architecture. | |||
There was a problem hiding this comment.
Recommending a verbose inline style for a common utility like 'visually hidden' content is suboptimal for maintainability. It is better to add a .sr-only utility class to app/globals.css once and reuse it across the project. This centralizes the logic and keeps the JSX clean. Additionally, the clip property is deprecated; clipPath: 'inset(50%)' is the modern standard.
| **Action:** When a screen-reader element needs to be visually hidden without breaking layout and no utility class exists, apply an explicit inline style pattern instead: `style={{ position: 'absolute', width: '1px', height: '1px', padding: 0, margin: '-1px', overflow: 'hidden', clip: 'rect(0, 0, 0, 0)', whiteSpace: 'nowrap', borderWidth: 0 }}`. This ensures accessibility guidelines are met within the constraints of the project architecture. | |
| **Action:** When a screen-reader element needs to be visually hidden, the preferred approach is to add a .sr-only utility class to app/globals.css. If inline styles must be used, apply a consistent pattern that avoids deprecated properties: style={{ position: 'absolute', width: '1px', height: '1px', padding: 0, margin: '-1px', overflow: 'hidden', clipPath: 'inset(50%)', whiteSpace: 'nowrap', border: 0 }}. This ensures accessibility guidelines are met while maintaining code quality. |
| {/* Optional: Raw Table for Details */} | ||
| <div style={{ overflowX: 'auto' }}> | ||
| <table style={{ width: '100%', borderCollapse: 'collapse' }}> | ||
| <caption style={{ position: 'absolute', width: '1px', height: '1px', padding: 0, margin: '-1px', overflow: 'hidden', clip: 'rect(0, 0, 0, 0)', whiteSpace: 'nowrap', borderWidth: 0 }}> |
There was a problem hiding this comment.
The clip property is deprecated in favor of clip-path. For better future-proofing, use clipPath: 'inset(50%)'. Furthermore, using inline styles for this common pattern is hard to maintain. Consider adding a .sr-only class to app/globals.css instead.
| <caption style={{ position: 'absolute', width: '1px', height: '1px', padding: 0, margin: '-1px', overflow: 'hidden', clip: 'rect(0, 0, 0, 0)', whiteSpace: 'nowrap', borderWidth: 0 }}> | |
| <caption style={{ position: 'absolute', width: '1px', height: '1px', padding: 0, margin: '-1px', overflow: 'hidden', clipPath: 'inset(50%)', whiteSpace: 'nowrap', border: 0 }}> |
💡 What
Added a visually hidden
<caption>element to the telemetry data table and appendedscope="col"to the table header cells (<th>). Added a journal entry detailing the approach for hiding elements visually without utility classes.🎯 Why
Screen-reader users had no context for the data table and no explicit association between column headers and data cells, causing a confusing navigational experience.
♿ Accessibility
<caption>tag.scope="col"..sr-onlyutility classes do not exist in this repo, explicitly styled the caption with inline CSS to guarantee accessibility while preserving visual layout.PR created automatically by Jules for task 4271447472720523932 started by @igor-holt