-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat(accessibility): enhance roadmap components with collapsible functionality and accessibility improvements #5286
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -36,6 +36,7 @@ interface IPillProps { | |
| isCollapsible?: boolean; | ||
| isCollapsed?: boolean; | ||
| onClickCollapse?: () => void; | ||
| collapsibleContentId?: string; | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -55,9 +56,45 @@ export default function Pill({ | |
| colorClass = '', | ||
| isCollapsible = false, | ||
| isCollapsed = false, | ||
| onClickCollapse = () => {} | ||
| onClickCollapse = () => {}, | ||
| collapsibleContentId | ||
| }: IPillProps) { | ||
| const [isDescriptionVisible, setIsDescriptionVisible] = useState(false); | ||
| const isDescriptionTrigger = !item.url && Boolean(item.description); | ||
| const interactiveClassName = | ||
| 'block rounded-md text-left font-medium text-gray-900 transition-colors hover:text-gray-600 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-violet focus-visible:ring-offset-2'; | ||
| const titleContent = ( | ||
| <> | ||
| {item.done && ( | ||
| <span title='Done!'> | ||
| <DoneIcon /> | ||
| </span> | ||
| )} | ||
| <span>{item.title}</span> | ||
| </> | ||
| ); | ||
|
|
||
| let titleElement = <span className='block text-left font-medium text-gray-900'>{item.title}</span>; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Keep the done icon in the non-interactive title path. The fallback branch renders only Suggested fix- let titleElement = <span className='block text-left font-medium text-gray-900'>{item.title}</span>;
+ let titleElement = <span className='block text-left font-medium text-gray-900'>{titleContent}</span>;🤖 Prompt for AI Agents |
||
|
|
||
| if (item.url) { | ||
| titleElement = ( | ||
| <a href={item.url} rel='noopener noreferrer' className={`${interactiveClassName} cursor-pointer`}> | ||
| {titleContent} | ||
| </a> | ||
| ); | ||
| } else if (isDescriptionTrigger) { | ||
| titleElement = ( | ||
| <button | ||
| type='button' | ||
| onClick={() => setIsDescriptionVisible(true)} | ||
| aria-label={`Open details for ${item.title}`} | ||
| aria-haspopup='dialog' | ||
| className={`${interactiveClassName} cursor-pointer`} | ||
| > | ||
| {titleContent} | ||
| </button> | ||
| ); | ||
| } | ||
|
|
||
| return ( | ||
| <> | ||
|
|
@@ -71,23 +108,17 @@ export default function Pill({ | |
| 'flex flex-1 items-center justify-between rounded-r-md border-y border-r border-gray-200 bg-white' | ||
| } | ||
| > | ||
| <div className='px-4 py-2 text-sm'> | ||
| <a | ||
| href={item.url} | ||
| rel='noopener noreferrer' | ||
| onClick={() => !item.url && item.description && setIsDescriptionVisible(true)} | ||
| className={`block text-left font-medium text-gray-900 ${item.description || item.url ? 'cursor-pointer hover:text-gray-600' : 'cursor-default'}`} | ||
| > | ||
| {item.done && ( | ||
| <span title='Done!'> | ||
| <DoneIcon /> | ||
| </span> | ||
| )} | ||
| <span>{item.title}</span> | ||
| </a> | ||
| </div> | ||
| <div className='px-4 py-2 text-sm'>{titleElement}</div> | ||
| {isCollapsible && ( | ||
| <button className='mr-2' onClick={onClickCollapse} data-testid='RoadmapItem-button'> | ||
| <button | ||
| type='button' | ||
| className='mr-2 rounded-md p-1 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-violet focus-visible:ring-offset-2' | ||
| onClick={onClickCollapse} | ||
| data-testid='RoadmapItem-button' | ||
| aria-expanded={!isCollapsed} | ||
| aria-controls={collapsibleContentId} | ||
| aria-label={`${isCollapsed ? 'Expand' : 'Collapse'} ${item.title}`} | ||
| > | ||
| <IconArrowRight className={`h-4 ${isCollapsed ? 'rotate-90' : '-rotate-90'}`} /> | ||
| </button> | ||
| )} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| import React from 'react'; | ||
| import { renderToStaticMarkup } from 'react-dom/server'; | ||
|
|
||
| import RoadmapItem from '../../components/roadmap/RoadmapItem'; | ||
| import RoadmapPill from '../../components/roadmap/RoadmapPill'; | ||
|
|
||
| describe('roadmap accessibility', () => { | ||
| test('renders description-only roadmap pills as accessible buttons', () => { | ||
| const markup = renderToStaticMarkup( | ||
| <RoadmapPill | ||
| item={{ | ||
| title: 'Accessible roadmap item', | ||
| description: 'Extra details' | ||
| }} | ||
| /> | ||
| ); | ||
|
|
||
| expect(markup).toContain('<button'); | ||
| expect(markup).toContain('type="button"'); | ||
| expect(markup).toContain('aria-label="Open details for Accessible roadmap item"'); | ||
| expect(markup).toContain('aria-haspopup="dialog"'); | ||
| expect(markup).toContain('focus-visible:ring-2'); | ||
| expect(markup).not.toContain('<a'); | ||
| }); | ||
|
|
||
| test('connects collapsible roadmap items to controlled content', () => { | ||
| const markup = renderToStaticMarkup( | ||
| <RoadmapItem | ||
| item={{ | ||
| title: 'Expandable roadmap item', | ||
| solutions: [{ title: 'Nested solution' }] | ||
| }} | ||
| colorClass='bg-blue-400' | ||
| /> | ||
| ); | ||
|
|
||
| const controlsMatch = markup.match(/aria-controls="([^"]+)"/); | ||
|
|
||
| expect(controlsMatch).not.toBeNull(); | ||
| expect(markup).toContain('aria-expanded="false"'); | ||
| expect(markup).toContain(`id="${controlsMatch?.[1]}"`); | ||
| expect(markup).toContain('aria-label="Expand Expandable roadmap item"'); | ||
| expect(markup).toContain('hidden=""'); | ||
| }); | ||
|
|
||
| test('marks collapsible roadmap items as expanded when children are visible', () => { | ||
| const markup = renderToStaticMarkup( | ||
| <RoadmapItem | ||
| item={{ | ||
| title: 'Expanded roadmap item', | ||
| solutions: [{ title: 'Visible child' }] | ||
| }} | ||
| colorClass='bg-blue-400' | ||
| collapsed={false} | ||
| /> | ||
| ); | ||
|
|
||
| expect(markup).toContain('aria-expanded="true"'); | ||
| expect(markup).toContain('aria-label="Collapse Expanded roadmap item"'); | ||
| expect(markup).toContain('Visible child'); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Base collapsibility on child count, not just field presence.
solutions: []orimplementations: []currently marks the item as collapsible, but the child lists only render when.lengthis truthy. That leaves an expand/collapse control wired to an empty container.Suggested fix
Also applies to: 53-61
🤖 Prompt for AI Agents