fix(slider): add accessible description context #3667
Conversation
|
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 📝 WalkthroughWalkthroughAdds aria-describedby to the slider input, referencing helperTextId when helperText is present. No other behavior or public API changes; aria-labelledby and aria-controls remain unchanged. Changes
Sequence Diagram(s)Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changesPossibly 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/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
|
Documentation has been published to https://lundalogik.github.io/lime-elements/versions/PR-3667/ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/components/slider/slider.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.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/slider/slider.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/slider/slider.tsx
**/*.{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/components/slider/slider.tsx
**/*.{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/slider/slider.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/slider/slider.tsx
⏰ 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 (1)
src/components/slider/slider.tsx (1)
256-258: a11y: Conditional aria-describedby looks correctGood: the input now references helper text only when present, preventing a dangling idref.
| aria-describedby={ | ||
| this.helperText ? this.helperTextId : undefined | ||
| } | ||
| aria-labelledby={this.labelId} | ||
| aria-controls={this.helperTextId} | ||
| {...inputProps} |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify cross-shadow ARIA: helperTextId must be on limel-helper-line host
If limel-helper-line assigns helperTextId to an element inside its own shadow root (instead of the host element), idref resolution from the input may fail across shadow boundaries. Ensure the id is on the limel-helper-line host (e.g., ) or another element in the same shadow tree as the input.
Run this script to confirm:
🏁 Script executed:
#!/bin/bash
# Locate the limel-helper-line component and verify where helperTextId is applied.
set -euo pipefail
echo "Searching for limel-helper-line implementation..."
files=$(fd -t f -g "src/**/helper-line*.tsx" || true)
if [ -z "${files}" ]; then
echo "No helper-line component found under src/**."
exit 1
fi
for f in $files; do
echo "=== $f ==="
rg -n -C2 "tag:\s*'limel-helper-line'" "$f" || true
rg -n -C3 "helperTextId" "$f" || true
rg -n -C3 "<Host\b.*id=" "$f" || true
rg -n -C3 "id=\{.*helperTextId" "$f" || true
done
echo
echo "Expectation:"
echo "- helperTextId should be bound to the limel-helper-line host (e.g., <Host id={this.helperTextId} />)"
echo " or another element that resides in the same shadow tree as the input."Length of output: 301
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "Searching for limel-helper-line component in src/components..."
files=$(rg -l "tag:\\s*['\"]limel-helper-line['\"]" -g "src/components/**/*.tsx" || true)
if [ -z "$files" ]; then
echo "limel-helper-line component not found under src/components."
exit 1
fi
for f in $files; do
echo "=== $f ==="
rg -n -C2 "helperTextId" "$f" || true
rg -n -C2 "<Host\b" "$f" || true
rg -n -C2 "id=\{.*helperTextId" "$f" || true
doneLength of output: 1216
Move the helperTextId onto the host element of limel-helper-line
In src/components/helper-line/helper-line.tsx, remove the id={this.helperTextId} from the internal <span> (around line 94) and instead add id={this.helperTextId} to the <Host> (around line 65). This ensures the ID lives on the host element so aria-describedby can resolve across shadow boundaries.
🤖 Prompt for AI Agents
In src/components/slider/slider.tsx around lines 256 to 261 and in
src/components/helper-line/helper-line.tsx (Host around line 65, internal <span>
around line 94): move the id={this.helperTextId} off the internal span and onto
the component host element so aria-describedby can resolve across shadow DOM;
remove the id attribute from the internal span, add id={this.helperTextId} to
the <Host> element, ensure the helperTextId property is still generated and
available on the host, and run/update any tests that reference the old span id
or query inside the shadow root.
|
🎉 This PR is included in version 38.23.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
fix: #3665
Summary by CodeRabbit
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: