Add accessible, localized labels for table selection checkboxes #3637
Conversation
|
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. Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThis change introduces accessible, localized labels for table selection checkboxes. It adds translation keys for "select all rows" and "select this row" in multiple languages, updates the table and checkbox components to use these labels, and visually hides the labels while keeping them accessible for screen readers. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Table
participant TableSelection
participant Checkbox
participant Translations
User->>Table: Render Table with language
Table->>Translations: getTranslation('table.select-all')
Translations-->>Table: Localized "Select all rows"
Table->>Checkbox: Render select-all Checkbox (label, class="hide-label")
User->>Checkbox: Interact (label visually hidden, accessible for screen readers)
Table->>TableSelection: Construct TableSelection(getTranslation)
TableSelection->>Translations: getTranslation('table.select-row')
Translations-->>TableSelection: Localized "Select this row"
TableSelection->>Checkbox: Render row Checkbox (label, class="hide-label")
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Possibly related PRs
Suggested labels
Suggested reviewers
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Pull Request Overview
This PR adds accessibility improvements to table row selection by providing screen reader support through translated labels for select-all and select-row checkboxes. The implementation maintains visual design while enhancing accessibility.
- Adds translation keys for table selection labels across 8 languages
- Implements a
languageprop for the table component to support internationalization - Introduces CSS styling to visually hide labels while keeping them accessible to screen readers
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/translations/*.ts | Adds translation keys for table selection labels in 8 languages |
| src/components/table/table.tsx | Adds language prop and integrates translation function for accessibility labels |
| src/components/table/table-selection.ts | Updates constructor to accept translation function and adds labels to checkboxes |
| src/components/table/table-selection.spec.ts | Updates test to include mock translation function |
| src/components/checkbox/checkbox.scss | Adds styling to hide labels visually while maintaining accessibility |
| etc/lime-elements.api.md | Updates API documentation to include new language property |
| import { TableSelection } from './table-selection'; | ||
| import { mapLayout, Layout } from './layout'; | ||
| import { areRowsEqual } from './utils'; | ||
| import { Languages } from '../date-picker/date.types'; |
There was a problem hiding this comment.
Importing Languages type from the date-picker component creates an unintended dependency. Consider moving this shared type to a common types file or creating a dedicated types file for language definitions.
coderabbitai
bot
changed the title
|
Documentation has been published to https://lundalogik.github.io/lime-elements/versions/PR-3637/ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
etc/lime-elements.api.mdis excluded by!etc/lime-elements.api.md
📒 Files selected for processing (12)
src/components/checkbox/checkbox.scss(1 hunks)src/components/table/table-selection.spec.ts(1 hunks)src/components/table/table-selection.ts(2 hunks)src/components/table/table.tsx(5 hunks)src/translations/da.ts(1 hunks)src/translations/de.ts(1 hunks)src/translations/en.ts(1 hunks)src/translations/fi.ts(1 hunks)src/translations/fr.ts(1 hunks)src/translations/nl.ts(1 hunks)src/translations/no.ts(1 hunks)src/translations/sv.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
⚙️ CodeRabbit Configuration File
**/*.{ts,tsx}: Imports from other files in the same module (lime-elements) must use relative paths. Using absolute paths for imports will cause the production build to fail.
Files:
src/translations/en.tssrc/translations/fi.tssrc/translations/fr.tssrc/components/table/table-selection.spec.tssrc/translations/de.tssrc/translations/no.tssrc/translations/nl.tssrc/translations/sv.tssrc/translations/da.tssrc/components/table/table-selection.tssrc/components/table/table.tsx
src/components/**/*.scss
📄 CodeRabbit Inference Engine (.cursor/rules/css-class-names-in-components-using-shadow-dom.mdc)
Do not use BEM-style class names in CSS for components, as components use shadow-DOM.
Files:
src/components/checkbox/checkbox.scss
**/*.{tsx,scss}
⚙️ CodeRabbit Configuration File
**/*.{tsx,scss}: Almost all our components use shadow-DOM. Therefore, we have no need of BEM-style class names in our CSS.
Files:
src/components/checkbox/checkbox.scsssrc/components/table/table.tsx
**/*.tsx
📄 CodeRabbit Inference Engine (.cursor/rules/wrap-multiple-jsx-elements-in-host-not-in-array.mdc)
When returning multiple JSX elements from the
rendermethod, never wrap them in an array literal. Instead, always wrap them in the special<Host>element. When there is already a single top-level element, it does not have to be wrapped in<Host>.
Files:
src/components/table/table.tsx
⚙️ CodeRabbit Configuration File
**/*.tsx: Our.tsxfiles are typically using StencilJS, not React. When a developer wants to return multiple top-level JSX elements from therendermethod, they will sometimes wrap them in an array literal. In these cases, rather than recommending they addkeyproperties to the elements, recommend removing the hardcoded array literal. Recommend wrapping the elements in StencilJS's special<Host>element.
Files:
src/components/table/table.tsx
src/components/**/*.tsx
⚙️ CodeRabbit Configuration File
src/components/**/*.tsx: When contributors add new props to existing components in the lime-elements repository, they should always add documentation examples that demonstrate the new prop's usage and explain how it works. This helps with user adoption, feature discoverability, and maintains comprehensive documentation.
Files:
src/components/table/table.tsx
🧠 Learnings (8)
📚 Learning: examples in lime elements use shadow dom (with `shadow: true` in the component decorator) for style ...
Learnt from: Kiarokh
PR: Lundalogik/lime-elements#3518
File: src/components/notched-outline/examples/notched-outline-basic.scss:11-22
Timestamp: 2025-04-14T10:01:18.721Z
Learning: Examples in Lime Elements use Shadow DOM (with `shadow: true` in the component decorator) for style encapsulation, making additional class-based selectors unnecessary. This ensures styles are automatically scoped to each component without leaking to the rest of the application.
Applied to files:
src/components/checkbox/checkbox.scss
📚 Learning: in components using shadow dom (like those in lime-elements), the universal selector `*` is already ...
Learnt from: adrianschmidt
PR: Lundalogik/lime-elements#3529
File: src/components/ai-avatar/ai-avatar.scss:20-22
Timestamp: 2025-04-29T14:15:32.104Z
Learning: In components using Shadow DOM (like those in lime-elements), the universal selector `*` is already scoped to the component's shadow tree and won't cause unwanted side effects outside the component. There's no functional difference between `*` and `:host *` in this context.
Applied to files:
src/components/checkbox/checkbox.scss
📚 Learning: applies to src/components/**/*.scss : do not use bem-style class names in css for components, as com...
Learnt from: CR
PR: Lundalogik/lime-elements#0
File: .cursor/rules/css-class-names-in-components-using-shadow-dom.mdc:0-0
Timestamp: 2025-07-21T00:27:58.489Z
Learning: Applies to src/components/**/*.scss : Do not use BEM-style class names in CSS for components, as components use shadow-DOM.
Applied to files:
src/components/checkbox/checkbox.scss
📚 Learning: in the lime-elements repository, css custom properties prefixed with `--lime-` are considered intern...
Learnt from: Kiarokh
PR: Lundalogik/lime-elements#3529
File: src/components/lime-ai-avatar/lime-ai-avatar.scss:1-3
Timestamp: 2025-04-25T14:23:56.018Z
Learning: In the lime-elements repository, CSS custom properties prefixed with `--lime-` are considered internal implementation details and should not be documented in JSDoc comments, while other custom properties should be documented as they are part of the public API.
Applied to files:
src/components/checkbox/checkbox.scss
📚 Learning: in lime-elements, css custom properties follow a specific naming convention: 1. internal use variabl...
Learnt from: Kiarokh
PR: Lundalogik/lime-elements#3529
File: src/components/lime-ai-avatar/lime-ai-avatar.scss:24-28
Timestamp: 2025-04-25T14:22:02.157Z
Learning: In lime-elements, CSS custom properties follow a specific naming convention:
1. Internal use variables (not for consumers) are prefixed with `--limel-component-name-`
2. External use variables (intended for consumers) are prefixed with `--component-name-`
Variables documented with JSDoc comments are typically meant for external use.
Applied to files:
src/components/checkbox/checkbox.scss
📚 Learning: in lime-elements, css custom properties like `--limel-theme-surface-background-color` are applied at...
Learnt from: Kiarokh
PR: Lundalogik/lime-elements#3624
File: src/components/button-group/button-group.scss:115-118
Timestamp: 2025-07-30T14:20:41.791Z
Learning: In lime-elements, CSS custom properties like `--limel-theme-surface-background-color` are applied at :root level via `core-styles.scss` and should be expected to be available without fallbacks. The library's architecture guarantees these theme variables are present, so adding fallbacks would be redundant and go against the design intention.
Applied to files:
src/components/checkbox/checkbox.scss
📚 Learning: in limel-chip-set, the onclick handler should be placed on the content div () ra...
Learnt from: Kiarokh
PR: Lundalogik/lime-elements#3518
File: src/components/chip-set/chip-set.tsx:367-385
Timestamp: 2025-04-14T12:22:22.298Z
Learning: In limel-chip-set, the onClick handler should be placed on the content div (<div slot="content">) rather than on the limel-notched-outline component, and should only be applied when type is 'input'.
Applied to files:
src/components/checkbox/checkbox.scss
📚 Learning: in lime elements components that use shadow dom (with `shadow: true` in the component decorator), ke...
Learnt from: Kiarokh
PR: Lundalogik/lime-elements#3529
File: src/components/lime-ai-avatar/lime-ai-avatar.scss:96-122
Timestamp: 2025-04-25T14:18:07.755Z
Learning: In Lime Elements components that use Shadow DOM (with `shadow: true` in the component decorator), keyframe animation names don't need to be namespaced as they are automatically scoped to the component and won't collide with animations from other components.
Applied to files:
src/components/checkbox/checkbox.scss
🧬 Code Graph Analysis (2)
src/components/table/table-selection.ts (4)
src/components/table/element-pool.ts (1)
ElementPool(1-67)src/components/table/table.types.ts (1)
RowData(217-219)src/components/file/file.tsx (1)
getTranslation(230-232)src/components/file-viewer/file-viewer.tsx (1)
getTranslation(444-446)
src/components/table/table.tsx (1)
src/components/date-picker/date.types.ts (1)
Languages(16-25)
⏰ 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: Docs / Publish Docs
- GitHub Check: Test
- GitHub Check: Build
🔇 Additional comments (18)
src/translations/en.ts (1)
53-54: Translation keys added correctly
'table.select-all'and'table.select-row'follow the existing naming pattern and use clear, concise phrasing.src/components/checkbox/checkbox.scss (2)
75-82: Confirm intent of reduced clickable area for hidden labelsSwitching
labelwidth from100%(default rule) to$box-sizeinside the.hide-labelscope shrinks the clickable target to just the checkbox, whereas the previous implementation allowed clicking the whole label area. This is a usability trade-off—screen-reader accessibility is kept, but pointer usability is reduced.Please double-check with design/UX that limiting the hit-area is intentional for row-selection checkboxes in
limel-table.
75-82: No BEM violations, rule looks solidClass name
.hide-labelis a simple modifier and does not conflict with the “no-BEM in Shadow DOM” guideline. The mixin usage and opacity technique are appropriate for visually hiding while preserving accessibility.src/translations/nl.ts (1)
55-56: Dutch translations look goodPhrasing is idiomatic and consistent with existing keys.
src/translations/fi.ts (1)
54-55: Finnish translations look goodKeys match the English source and grammar is correct.
src/translations/de.ts (1)
55-56: German translations look goodTerminology aligns with current UI wording.
src/translations/no.ts (1)
53-55: ✓ Norwegian keys added and phrased correctlyThe new keys follow the existing naming convention and grammar looks good.
src/translations/sv.ts (1)
53-55: ✓ Swedish translations look accurate and consistentNo issues spotted; key names and phrasing align with the rest of the file.
src/translations/fr.ts (1)
55-57: ✓ French translations added correctlyKeys and language usage are sound and match conventions.
src/translations/da.ts (1)
53-55: ✓ Danish translations added correctlyKeys are consistent; spelling and diacritics look correct.
src/components/table/table-selection.ts (2)
24-30: LGTM! Constructor properly extended with translation support.The constructor documentation and signature correctly add the
getTranslationparameter with proper typing and documentation following the established patterns.
111-113: LGTM! Accessibility improvements properly implemented.The addition of localized labels and the
hide-labelCSS class correctly addresses the accessibility issue while maintaining the visual design. The translation key'table.select-row'is appropriate for row selection checkboxes.src/components/table/table.tsx (6)
25-26: LGTM! Imports follow coding guidelines.The relative path imports comply with the coding guidelines for files in the same module.
163-167: LGTM! Language property properly implemented.The property follows established patterns with proper documentation, typing (
Languages), sensible default value ('en'), and appropriate reflection as an attribute.
480-481: LGTM! TableSelection constructor properly updated.The constructor call correctly passes the translation function callback, integrating well with the updated
TableSelectionclass.
864-872: LGTM! Select-all checkbox accessibility properly implemented.The addition of the
hide-labelclass and localized label using'table.select-all'translation key correctly addresses accessibility requirements while maintaining the visual design.
892-894: LGTM! Translation method implemented consistently.The
getTranslationmethod follows the same pattern used in other components (file.tsx, file-viewer.tsx) and correctly utilizes the translate utility with the component's language property.
872-872: Translation keys verified across all languagesAll supported translation files (
da.ts,de.ts,en.ts,fi.ts,fr.ts,nl.ts,no.ts,sv.ts) include both
table.select-alltable.select-rowNo missing translations detected.
No further action required.
Kiarokh
force-pushed
the
checkbox-label-table
branch
from
78fbdcd to
3c076dd
Compare
We need a helper class to be used internally by components such as `limel-table` to hide the `<label>`, while still keeping the checkbox both clickable for the users, and accessible for screen readers.
Separating this commit for an easier review. Here is what's added: 1. "Select all rows" 2. "Select this row"
Kiarokh
force-pushed
the
checkbox-label-table
branch
from
1531448 to
84270e7
Compare
|
🎉 This PR is included in version 38.21.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
fix #3636
Summary by CodeRabbit
New Features
Style
Bug Fixes
Review:
Browsers tested:
(Check any that applies, it's ok to leave boxes unchecked if testing something didn't seem relevant.)
Windows:
Linux:
macOS:
Mobile: