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

fix(@clayui/pagination): LPD-1285 accessibility issues on prev, next … #5779

Merged
merged 2 commits into from
Mar 8, 2024
Merged

fix(@clayui/pagination): LPD-1285 accessibility issues on prev, next … #5779

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
640dd96
fix(@clayui/pagination): LPD-1285 accessibility issues on prev, next …
pat270 Mar 5, 2024
0968e4b
chore(@clayui/pagination): LPD-1285 regen snapshots
pat270 Mar 6, 2024
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
2 changes: 1 addition & 1 deletion packages/clay-core/src/drop-down/Menu.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ function MenuInner<T extends Record<string, unknown> | string | number>(
return (
<div className="dropdown" ref={containerRef}>
{React.cloneElement(trigger, {
'aria-controls': ariaControlsId,
'aria-controls': active ? ariaControlsId : undefined,
'aria-expanded': active,
'aria-haspopup': 'true',
className: classNames(
Expand Down
4 changes: 0 additions & 4 deletions packages/clay-core/src/table/__tests__/__snapshots__/BasicRendering.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ exports[`Table basic rendering render dynamic content 1`] = `
class="dropdown"
>
<button
aria-controls="clay-id-12"
aria-haspopup="true"
aria-label="Manage Columns Visibility"
class="dropdown-toggle btn btn-monospaced btn-outline-borderless btn-outline-secondary"
Expand Down Expand Up @@ -193,7 +192,6 @@ exports[`Table basic rendering render static content 1`] = `
class="dropdown"
>
<button
aria-controls="clay-id-4"
aria-haspopup="true"
aria-label="Manage Columns Visibility"
class="dropdown-toggle btn btn-monospaced btn-outline-borderless btn-outline-secondary"
Expand Down Expand Up @@ -423,7 +421,6 @@ exports[`Table basic rendering render with sort column 1`] = `
class="dropdown"
>
<button
aria-controls="clay-id-20"
aria-haspopup="true"
aria-label="Manage Columns Visibility"
class="dropdown-toggle btn btn-monospaced btn-outline-borderless btn-outline-secondary"
Expand Down Expand Up @@ -584,7 +581,6 @@ exports[`Table basic rendering render with treegrid 1`] = `
class="dropdown"
>
<button
aria-controls="clay-id-28"
aria-haspopup="true"
aria-label="Manage Columns Visibility"
class="dropdown-toggle btn btn-monospaced btn-outline-borderless btn-outline-secondary"
Expand Down
4 changes: 2 additions & 2 deletions packages/clay-pagination-bar/src/__tests__/__snapshots__/index.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@ exports[`ClayPaginationBar renders 1`] = `
class="dropdown"
>
<button
aria-controls="clay-id-5"
aria-haspopup="true"
aria-label="Show pages 4 through 9"
class="dropdown-toggle page-link btn btn-unstyled"
Expand Down Expand Up @@ -128,6 +127,7 @@ exports[`ClayPaginationBar renders 1`] = `
aria-label="Go to the next page, 2"
class="page-link"
data-testid="nextArrow"
role="button"
tabindex="0"
>
<svg
Expand Down Expand Up @@ -220,7 +220,6 @@ exports[`ClayPaginationBar renders without a DropDown 1`] = `
class="dropdown"
>
<button
aria-controls="clay-id-8"
aria-haspopup="true"
aria-label="Show pages 4 through 9"
class="dropdown-toggle page-link btn btn-unstyled"
Expand Down Expand Up @@ -249,6 +248,7 @@ exports[`ClayPaginationBar renders without a DropDown 1`] = `
aria-label="Go to the next page, 2"
class="page-link"
data-testid="nextArrow"
role="button"
tabindex="0"
>
<svg
Expand Down
10 changes: 10 additions & 0 deletions packages/clay-pagination/src/PaginationWithBasicItems.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,11 @@ const ClayPaginationWithBasicItems = React.forwardRef<HTMLUListElement, IProps>(
disabled={internalActive === 1}
href={previousHref}
onClick={() => setActive(previousPage)}
role={
previousHref || internalActive === 1
Copy link
Member

Choose a reason for hiding this comment

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

If there is a previousHref, it will be a string which would be a truthy value. Then this would be set as the role, right? So I don't think this logic wouldn't be correct when previousHref exists.

Copy link
Member Author

Choose a reason for hiding this comment

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

If previousHref exists, the pagination component outputs <a class="page-link" href="/url">. In this case, we don't need role="button" because it's a hyperlink.

Copy link
Member

Choose a reason for hiding this comment

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

Okay... looks good. Sorry I was incorrectly interpreting the precedence of the operators.

? undefined
: 'button'
}
>
<ClayIcon spritemap={spritemap} symbol="angle-left" />
</Pagination.Item>
Expand Down Expand Up @@ -217,6 +222,11 @@ const ClayPaginationWithBasicItems = React.forwardRef<HTMLUListElement, IProps>(
disabled={internalActive === totalPages}
href={nextHref}
onClick={() => setActive(nextPage)}
role={
nextHref || internalActive === totalPages
? undefined
: 'button'
}
>
<ClayIcon spritemap={spritemap} symbol="angle-right" />
</Pagination.Item>
Expand Down
4 changes: 2 additions & 2 deletions packages/clay-pagination/src/__tests__/__snapshots__/index.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ exports[`ClayPagination renders 1`] = `
aria-label="Go to the previous page, 11"
class="page-link"
data-testid="prevArrow"
role="button"
tabindex="0"
>
<svg
Expand Down Expand Up @@ -45,7 +46,6 @@ exports[`ClayPagination renders 1`] = `
class="dropdown"
>
<button
aria-controls="clay-id-2"
aria-haspopup="true"
aria-label="Show pages 2 through 9"
class="dropdown-toggle page-link btn btn-unstyled"
Expand Down Expand Up @@ -118,7 +118,6 @@ exports[`ClayPagination renders 1`] = `
class="dropdown"
>
<button
aria-controls="clay-id-4"
aria-haspopup="true"
aria-label="Show pages 15 through 24"
class="dropdown-toggle page-link btn btn-unstyled"
Expand Down Expand Up @@ -147,6 +146,7 @@ exports[`ClayPagination renders 1`] = `
aria-label="Go to the next page, 13"
class="page-link"
data-testid="nextArrow"
role="button"
tabindex="0"
>
<svg
Expand Down
Loading