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.

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

[EuiTablePagination] Add aria-current to rows per page menu #7186

Merged
merged 9 commits into from
Sep 14, 2023

Conversation

1Copenut
Copy link
Contributor

@1Copenut 1Copenut commented Sep 13, 2023

Summary

This PR adds an aria-current attribute to the current page for EuiTable, EuiDataGrid, and nested EuiContextMenu used for rows per page. PR closes #6649.

QA

Remove or strikethrough items that do not apply to your PR.

General checklist

  • Browser QA
    • Checked in both light and dark modes
    • Checked in mobile
    • Checked in Chrome, Safari, Edge, and Firefox
    • Checked for accessibility including keyboard-only and screenreader modes
  • Docs site QA
  • Code quality checklist
  • Release checklist
    • A changelog entry exists and is marked appropriately.
    • If applicable, added the breaking change issue label (and filled out the breaking change checklist)
  • Designer checklist
    • Updated the Figma library counterpart

@1Copenut 1Copenut changed the title [EuiTablePagination] Add aria-current to current page and rows per page menu [EuiTable] Add aria-current to current pagination button and rows per page menu Sep 13, 2023
@1Copenut 1Copenut self-assigned this Sep 13, 2023
@1Copenut 1Copenut marked this pull request as ready for review September 13, 2023 21:37
@cee-chen cee-chen changed the title [EuiTable] Add aria-current to current pagination button and rows per page menu [EuiTablePagination] Add aria-current to current pagination button and rows per page menu Sep 13, 2023
@@ -114,8 +114,11 @@ export const EuiTablePagination: FunctionComponent<EuiTablePaginationProps> = (
() =>
itemsPerPageOptions.map((itemsPerPageOption) => (
<EuiContextMenuItem
key={itemsPerPageOption}
Copy link
Member

@cee-chen cee-chen Sep 14, 2023

Choose a reason for hiding this comment

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

This key is still needed for .map iteration. You should be see a ton of warnings in the console without it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I re-added the icon and aria-current before the key, so the default PR view shows it removed. The key is still in the code on L#121. I can move it back to previous location.

Copy link
Member

Choose a reason for hiding this comment

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

oh whoops, you're right 🤦 yeah let's make the key the first prop as a general rule, it makes it easier to tell it's there since it's a "special" prop

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like that as an EUI house style and sound argument. I'll move it to the first prop declaration and set auto-merge after the tests run.

@cee-chen cee-chen changed the title [EuiTablePagination] Add aria-current to current pagination button and rows per page menu [EuiTablePagination] Add aria-current to rows per page menu Sep 14, 2023
Copy link
Member

@cee-chen cee-chen left a comment

Choose a reason for hiding this comment

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

Approving - key prop move is an optional change request. Thanks Trevor!

@1Copenut 1Copenut enabled auto-merge (squash) September 14, 2023 16:52
@kibanamachine
Copy link

Preview staging links for this PR:

@1Copenut 1Copenut merged commit 0f42263 into elastic:main Sep 14, 2023
7 checks passed
@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

cc @1Copenut

@1Copenut 1Copenut deleted the feature/euiPagination-aria-current branch September 20, 2023 17:47
jbudz pushed a commit to elastic/kibana that referenced this pull request Sep 27, 2023
`v88.3.0`⏩`v88.5.0`

closes #151514

---

## [`88.5.0`](https://github.com/elastic/eui/tree/v88.5.0)

- Updated `EuiCallOut` with a new `onDismiss` prop
([#7156](elastic/eui#7156))
- Added a new `renderCustomToolbar` prop to `EuiDataGrid`, which allows
custom rendering of the toolbar.
([#7190](elastic/eui#7190))
- Added a new `allowResetButton` prop to
`toolbarVisibility.showDisplaySelector` of `EuiDataGrid`, which allows
hiding the "Reset to default" button from the display settings popover.
([#7190](elastic/eui#7190))
- Added a new `additionalDisplaySettings` prop to
`toolbarVisibility.showDisplaySelector` of `EuiDataGrid`, which allows
rendering extra settings inside the display settings popover.
([#7190](elastic/eui#7190))
- Updated `EuiDataGrid`'s toolbar display settings button icon
([#7190](elastic/eui#7190))
- Updated `EuiTextTruncate` with significantly improved iteration
performance. Removed `measurementRenderAPI` prop, as `EuiTextTruncation`
now only uses more performant canvas render API
([#7210](elastic/eui#7210))
- Updated `EuiPopover` with a new configurable `repositionToCrossAxis`
prop ([#7211](elastic/eui#7211))
- Updated `EuiDatePicker` to support `compressed` input styling
([#7218](elastic/eui#7218))
- Added `gradient` and `palette` icon glyphs.
([#7220](elastic/eui#7220))

**Bug fixes**

- Fixed `EuiPopover`'s missing animations on popover close
([#7211](elastic/eui#7211))
- Fixed `EuiInputPopover` anchoring to the wrong side and missing
shadows on smaller screens
([#7211](elastic/eui#7211))
- Fixed `EuiSuperDatePicker` icon spacing on the quick select button
([#7217](elastic/eui#7217))
- Fixed a missing type in `EuiMarkdownEditor`'s default processing
plugins ([#7221](elastic/eui#7221))


## [`88.4.1`](https://github.com/elastic/eui/tree/v88.4.1)

**Bug fixes**

- Fixed missing `className`s on `EuiTextTruncate`
([#7212](elastic/eui#7212))
- Fixed `title`s on `EuiComboBox` dropdown options to always be present
([#7212](elastic/eui#7212))
- Fixed `EuiComboBox` truncation issues when search is an empty space
([#7212](elastic/eui#7212))

## [`88.4.0`](https://github.com/elastic/eui/tree/v88.4.0)

- Updated `EuiComboBox` to allow configuring text truncation behavior
via `truncationProps`. These props can be set on the entire combobox as
well as on on individual dropdown options.
([#7028](elastic/eui#7028))
- Updated `EuiInMemoryTable` with a new `searchFormat` prop (defaults to
`eql`). When setting this prop to `text`, the built-in search bar will
ignore EQL syntax and allow searching for plain strings with special
characters and symbols.
([#7175](elastic/eui#7175))

**Bug fixes**

- `EuiComboBox` now always shows the highlighted search text, even on
truncated text ([#7028](elastic/eui#7028))
- Fixed missing i18n in `EuiSearchBar`'s default placeholder and
aria-label text ([#7175](elastic/eui#7175))
- Fixed the inline compressed styles of `EuiDescriptionListTitle` to use
a taller line-height for readability
([#7185](elastic/eui#7185))
- Fixed `EuiComboBox` to correctly truncate selected items when
displayed as pills and plain text
([#7193](elastic/eui#7193))

**Accessibility**

- Added `aria-current` attribute to `EuiTablePagination`
([#7186](elastic/eui#7186))

**CSS-in-JS conversions**

- Converted `EuiDroppable` and `EuiDraggable` to Emotion; Removed
`$euiDragAndDropSpacing` Sass variables
([#7187](elastic/eui#7187))

---------

Co-authored-by: Patryk Kopycinski <contact@patrykkopycinski.com>
Co-authored-by: Jan Monschke <jan.monschke@elastic.co>
Co-authored-by: Thomas Watson <watson@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[EuiTablePagination] Update pagination with context menu that announces selected row count
4 participants