Add tooltip to checkboxes#125
Conversation
jivey
left a comment
jivey
left a comment
There was a problem hiding this comment.
One small issue, I'm seeing the tooltip read twice in a screenreader (maybe we have the same issue on the radio component?) I wonder if you need the aria-describedby to be on the input itself.
|
@jivey Ahh Ok I'll fix that. I'm curious, what does your screen reader setup look like? I would like to set one up as well. |
|
@P-Gill97 I tested this particular thing on Windows 11 with NVDA inside of a Parallels container, but I want to start using VoiceOver, I heard it's much better these days and it's definitely faster than firing up a VM. I'm also trying to get used to the Accessibility tab in chrome, though in this case I may not have noticed contents + description being an issue: 
Which is why I prefer checking with NVDA/VoiceOver so I don't have guess how it'll actually be read. |
There was a problem hiding this comment.
Pull request overview
This PR adds tooltip support to checkbox inputs, aligning their behavior with the existing radio tooltip pattern by introducing shared tooltip-trigger logic in the components layer.
Changes:
- Introduces a shared
useLabelTooltiphook inTooltip.tsxfor label-wrapped inputs. - Updates
Radioto use the shared tooltip hook instead of its bespoke tooltip state/trigger logic. - Updates
Checkboxto supporttooltipTextand adds a Storybook example; updates related snapshots.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/components/Tree/snapshots/Tree.spec.tsx.snap | Snapshot update due to styled-components class name changes. |
| src/components/Tooltip.tsx | Adds shared useLabelTooltip hook; adjusts tooltip-related imports. |
| src/components/Radio.tsx | Refactors radio tooltip behavior to use useLabelTooltip. |
| src/components/Checkbox/Checkbox.tsx | Adds tooltipText support and tooltip rendering path for checkboxes. |
| src/components/Checkbox/Checkbox.stories.tsx | Adds a WithTooltip Storybook example for checkboxes. |
| src/components/Checkbox/snapshots/Checkbox.spec.tsx.snap | Snapshot update due to styled-components class name changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -1,8 +1,10 @@ | |||
| import { useRef } from 'react'; | |||
There was a problem hiding this comment.
npx tsc --noEmit passes clean. False positive.
| const { triggerProps, tooltipProps } = useTooltipTrigger({ delay: 0 }, state, triggerRef); | ||
|
|
||
| const { 'aria-describedby': _unusedDescribedBy, ...labelTriggerProps } = triggerProps; | ||
|
|
||
| return { | ||
| triggerRef, | ||
| triggerProps: labelTriggerProps, | ||
| labelDescription: tooltipText ? <VisuallyHidden>{tooltipText}</VisuallyHidden> : null, | ||
| tooltip: tooltipText && state.isOpen | ||
| ? <CustomTooltip state={state} {...tooltipProps} placement={placement} aria-hidden={true}>{tooltipText}</CustomTooltip> | ||
| : null, | ||
| }; |
There was a problem hiding this comment.
stripping aria-describedby does not prevent assistive tech from announcing the tooltip. Also a disabled input isn't focusable, and describedby only speaks on focus so disabled checkboxes and radio inputs tooltips aren't announced.
| export const Checkbox = ({ children, disabled, variant = 'primary', bold = false, size = 1.6, labelProps, tooltipText, ...props }: CheckboxProps) => { | ||
| const { triggerRef, triggerProps, labelDescription, tooltip } = useLabelTooltip(tooltipText); | ||
|
|
||
| return tooltipText | ||
| ? <LabelWithTooltipWrapper> | ||
| <StyledLabel ref={triggerRef} bold={bold} variant={variant} isDisabled={disabled} {...triggerProps} {...labelProps}> | ||
| <StyledInput {...props} type="checkbox" variant={variant} checkboxSize={size} isDisabled={disabled} disabled={disabled} /> | ||
| {children} | ||
| {labelDescription} | ||
| </StyledLabel> | ||
| {tooltip} | ||
| </LabelWithTooltipWrapper> |
There was a problem hiding this comment.
tooltip behavior has not changed. Visually inspected and used VoiceOver to check accessibility.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
@jivey Requirements changed on the issue I was adding these tooltips to checkboxes for. I think I'm gonna hold off on working on this PR for a bit I got it most of the way there and fixed the double read issue also made the tooltips work correctly on disabled boxes. I will keep PR open because this is something we should add. |
https://openstax.atlassian.net/browse/CORE-2444
Update checkboxes to have tooltip like the radio buttons do. I needed this for the unlimited assignment attempts settings page. The tooltips will be defined in assessments
assessments/packages/lambda/src/functions/serviceApi/versions/v0/routes/integration.ts