Skip to content
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.

Sign up for GitHub

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

[BpkCalendar] Accessibility roles #3699

Merged
merged 5 commits into from
Jan 13, 2025
Merged

[BpkCalendar] Accessibility roles #3699

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f99a0cc
feat: make Calendar conform to grid for easier navigation
gert-janvercauteren Jan 2, 2025
6031d50
chore: remove commented code
gert-janvercauteren Jan 2, 2025
a6316da
fix: remove unused prop
gert-janvercauteren Jan 2, 2025
a748bc8
fix failing tests since disabled dates are no longer aria-hidden
gert-janvercauteren Jan 2, 2025
bd74da2
Merge branch 'main' into fix/calendar_a11y_roles
gert-janvercauteren Jan 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions packages/bpk-component-calendar/src/BpkCalendarContainer-test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ describe('BpkCalendarContainer', () => {

// dates in March are outside current month
const outsideDate = screen.getByRole('button', {
name: /1st March 2010/i,
name: /Monday, 1st March/i,
});
waitFor(() =>
expect(outsideDate.classList.contains('bpk-calendar-date--outside')).toBe(
Expand All @@ -125,7 +125,7 @@ describe('BpkCalendarContainer', () => {

// dates in March are within current month
const currentDate = screen.getByRole('button', {
name: /1st March 2010/i,
name: /Monday, 1st March/i,
});
waitFor(() =>
expect(currentDate.classList.contains('bpk-calendar-date--outside')).toBe(
Expand Down Expand Up @@ -300,20 +300,20 @@ describe('BpkCalendarContainer', () => {

await fireEvent.keyDown(getDate(originStr), { key: 'ArrowRight' });
expect(
getDate(/2nd March/).classList.contains('bpk-calendar-date--focused'),
getDate(/Tuesday, 2nd March/).classList.contains('bpk-calendar-date--focused'),
).toBe(true);

await fireEvent.keyDown(getDate(/2nd March/i), { key: 'ArrowDown' });
await fireEvent.keyDown(getDate(/Tuesday, 2nd March/i), { key: 'ArrowDown' });
expect(
getDate(/9th March/i).classList.contains('bpk-calendar-date--focused'),
getDate(/Tuesday, 9th March/i).classList.contains('bpk-calendar-date--focused'),
).toBe(true);

await fireEvent.keyDown(getDate(/9th March/i), { key: 'ArrowLeft' });
await fireEvent.keyDown(getDate(/Tuesday, 9th March/i), { key: 'ArrowLeft' });
expect(
getDate(/8th March/i).classList.contains('bpk-calendar-date--focused'),
getDate(/Monday, 8th March/i).classList.contains('bpk-calendar-date--focused'),
).toBe(true);

await fireEvent.keyDown(getDate(/8th March/i), { key: 'ArrowUp' });
await fireEvent.keyDown(getDate(/Monday, 8th March/i), { key: 'ArrowUp' });
expect(
getDate(originStr).classList.contains('bpk-calendar-date--focused'),
).toBe(true);
Expand Down Expand Up @@ -378,7 +378,7 @@ describe('BpkCalendarContainer', () => {

await fireEvent.keyDown(getDate(/28th February/i), { key: 'ArrowRight' });
expect(
getDate(/1st March/i).classList.contains('bpk-calendar-date--focused'),
getDate(/Monday, 1st March/i).classList.contains('bpk-calendar-date--focused'),
).toBe(true);
expect(onMonthChange.mock.calls.length).toEqual(1);
expect(onMonthChange.mock.calls[0][1]).toEqual({
Expand Down
1 change: 0 additions & 1 deletion packages/bpk-component-calendar/src/BpkCalendarDate.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,6 @@ class BpkCalendarDate extends PureComponent<Props> {
type="button"
style={style}
className={classNames.join(' ')}
aria-hidden={isBlocked}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this might be causing the blocked dates in the scrollable calendar to be read by the screen reader which doesn't seem to be happening on the main branch 🤔

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree with this point, currently the calendar wouldn't allow a AT user to go to dates that are in the past and make them inaccessible so with this removal this now allows them to select previous dates

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess are we happy with users being able to get to that date and with it mentioning 'dimmed' this is ok?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had a chat with Gert about this - it's not possible to keep this with the role of grid as it completely messes it up. Adding grid though brings a lot of benefits though like navigating up and down so even though you can now navigate through the blocked dates, it will be at most 30 days blocked, but you can just move down through the calendar so it's quick

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I purposefully unblocked the dimmed dates as otherwise, Grid navigation wouldn't work correctly :(

aria-label={`${date.getDate()}`}
disabled={isBlocked}
tabIndex={isKeyboardFocusable && isFocused ? 0 : -1}
Expand Down
4 changes: 2 additions & 2 deletions packages/bpk-component-calendar/src/BpkCalendarGrid.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,8 +181,8 @@ class BpkCalendarGrid extends Component<Props, State> {
const classNames = getClassName('bpk-calendar-grid', className);

return (
<div className={classNames} aria-hidden={!isKeyboardFocusable}>
<div>
<div className={classNames} aria-hidden={!isKeyboardFocusable} role="grid" >
<div role="rowgroup">
{calendarMonthWeeks.map((dates) => (
<BpkCalendarWeek
key={dates[0].isoLabel}
Expand Down
7 changes: 2 additions & 5 deletions packages/bpk-component-calendar/src/BpkCalendarWeek.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ class BpkCalendarWeek extends Component<Props> {
}

return (
<div className={getClassName('bpk-calendar-week')}>
<div className={getClassName('bpk-calendar-week')} role="row">
{this.props.dates.map((date) => {
const isBlocked =
minDate && maxDate
Expand All @@ -404,7 +404,6 @@ class BpkCalendarWeek extends Component<Props> {
<DateContainer
className={cellClassName}
isEmptyCell={!isSameMonth(date.val, month) && ignoreOutsideDate!}
isBlocked={isBlocked}
key={date.val.getDate()}
selectionType={dateSelectionType}
>
Expand Down Expand Up @@ -436,7 +435,6 @@ class BpkCalendarWeek extends Component<Props> {
type DateContainerProps = {
children: ReactElement;
className?: string | null;
isBlocked: boolean;
isEmptyCell: boolean;
selectionType: string;
};
Expand All @@ -446,7 +444,6 @@ type DateContainerProps = {
const DateContainer = ({
children,
className = null,
isBlocked,
isEmptyCell,
selectionType,
}: DateContainerProps) => {
Expand All @@ -457,7 +454,7 @@ const DateContainer = ({
);

return (
<div aria-hidden={isEmptyCell || isBlocked} className={classNames}>
<div aria-hidden={isEmptyCell} className={classNames} role="gridcell">
{children}
</div>
);
Expand Down
Loading
Loading