fix: add hint tooltips for quadrant axes#2419
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Lunaria Status Overview🌕 This pull request will trigger status changes. Learn moreBy default, every PR changing files present in the Lunaria configuration's You can change this by adding one of the keywords present in the Tracked Files
Warnings reference
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe pull request adds two SVG Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/components/Compare/FacetQuadrantChart.vue (1)
386-437: Well-implemented tooltip overlays for axis explanations.The use of
foreignObjectwithoverflow: visibleis appropriate for embedding interactive HTML tooltips within the SVG context. Good choices:
- The
interactiveprop onTooltipAppcorrectly enables pointer events and provides a 150ms hide delay, allowing users to move their cursor to the tooltip content.data-dom-to-png-ignoreappropriately excludes these UI controls from image exports.- Proper accessibility via
aria-labelattributes on the buttons.One minor formatting observation: the indentation within the
foreignObjectelements is inconsistent (e.g., lines 394-410 use different indentation than the surrounding template code). Consider aligning with the file's existing indentation style for consistency.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fa7ca62f-bc2e-4019-b116-e0ab752102bf
📒 Files selected for processing (4)
app/components/Compare/FacetQuadrantChart.vuei18n/locales/en.jsoni18n/locales/fr-FR.jsoni18n/schema.json
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ce78f7ba-eaae-476d-89f1-69daa28116b5
📒 Files selected for processing (1)
app/components/Compare/FacetQuadrantChart.vue
There was a problem hiding this comment.
♻️ Duplicate comments (1)
app/components/Compare/FacetQuadrantChart.vue (1)
390-394:⚠️ Potential issue | 🟠 MajorAdd accessible names to the new axis help buttons.
Both triggers are icon-only. Unlike the chart-level help button on Line 343, these buttons have no localised
aria-label, so screen readers will announce them as an unlabelled button and the new help text is not discoverable.♿ Suggested fix
<TooltipApp> <button data-dom-to-png-ignore class="i-lucide:info w-3.5 h-3.5 text-fg-muted cursor-help" type="button" + :aria-label="$t('compare.quadrant_chart.explanation.efficiency')" /> @@ <TooltipApp> <button data-dom-to-png-ignore class="i-lucide:info w-3.5 h-3.5 text-fg-muted cursor-help" type="button" + :aria-label="$t('compare.quadrant_chart.explanation.adoption')" />Also applies to: 415-419
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4ec96272-60c6-46eb-b0cb-ee6700270c33
📒 Files selected for processing (1)
app/components/Compare/FacetQuadrantChart.vue
serhalp
left a comment
There was a problem hiding this comment.
LGTM but there's an a11y violation 👀
I know it's an impossible one. I had to filter some violations just for this component. Hope it's ok, if not, we'll have to find another way. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Follow up to #2416
Compare page, quadrant chart.
This adds tooltip hints for the quadrant axes.
The hint icon is hidden for prints.