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

[EuiDataGrid] Add title attribute to column header cell content #6013

Merged
merged 4 commits into from
Jun 30, 2022

Conversation

cee-chen
Copy link
Contributor

@cee-chen cee-chen commented Jun 29, 2022

Summary

closes #6012

This uses displayAsText by default, falling back to id (which means title should always be present).

Checklist

  • Checked in Chrome, Safari, Edge, and Firefox
  • Props have proper autodocs and playground toggles
  • Added documentation
  • Added or updated jest and cypress tests snapshots
  • Checked for accessibility including keyboard-only and screenreader modes
  • A changelog entry exists and is marked appropriately

- [ ] Checked in both light and dark modes
- [ ] Checked in mobile
- [ ] Checked Code Sandbox works for any docs examples
- [ ] Checked for breaking changes and labeled appropriately
- [ ] Updated the Figma library counterpart

cee-chen added 2 commits June 29, 2022 10:27
- to make headings more visible if text is being truncated due to column widths
- clarify props documentation for `displayAsText`
- reorder props by general purpose
- Update `columns` snippet to include all props and also to follow new grouping order
@cee-chen cee-chen requested a review from chandlerprall June 29, 2022 18:46
@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_6013/

@cee-chen cee-chen marked this pull request as ready for review June 29, 2022 20:44
- reproducible on the Version column in the main datagrid example
@cee-chen cee-chen changed the title [EuiDataGrid] Add title attribute to column header cells [EuiDataGrid] Add title attribute to column header cell text Jun 29, 2022
@cee-chen cee-chen changed the title [EuiDataGrid] Add title attribute to column header cell text [EuiDataGrid] Add title attribute to column header cell content Jun 29, 2022
@cee-chen
Copy link
Contributor Author

cee-chen commented Jun 29, 2022

I did some testing on VO and it turns out leaving the title on the top-level th/cell header element causes VO to read out both the title and the content in duplicate, but only for column headers with actions disabled (this is a whole thing I'm figuring out with Trevor right now but the tl;dr is it's because the popover button is automatically focused by our cell focus logic).

In any case, putting the title on .euiDataGridHeaderCell__content instead of on the top-level role="columnheader" element solves the issue, so I went ahead and made that change.

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_6013/

Copy link
Contributor

@chandlerprall chandlerprall left a comment

Choose a reason for hiding this comment

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

Changes LGTM, tested in the PR's deployed docs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[EuiDataGrid] Add title attribute to header elements
3 participants