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

Add headerRow to EuiBasicTable #2802

Merged
merged 9 commits into from
Jan 31, 2020

Conversation

andreadelrio
Copy link
Contributor

@andreadelrio andreadelrio commented Jan 30, 2020

Summary

This PR adds a new prop called headerRow to BasicTable that lets consumers specify what column is the row header (and thus should have <th scope=“row”>).

Doing something like

    <EuiBasicTable
      items={items}
      rowHeader="firstName"
      columns={columns}
      rowProps={getRowProps}
      cellProps={getCellProps}
    />

results in this markup

Pasted Graphic 7 copy

Limitations:

This only works with FieldDataColumn as it has field as a required prop which is what I’m using for doing

(column as EuiTableFieldDataColumnType<T>).field === headerRow

Fixes #2335 #2337

Checklist

- [ ] Check against all themes for compatibility in both light and dark modes
- [ ] Checked in mobile
- [ ] Checked in IE11 and Firefox

  • Props have proper autodocs
  • Added documentation examples
  • Added or updated jest tests
    - [ ] Checked for breaking changes and labeled appropriately
    - [ ] Checked for accessibility including keyboard-only and screenreader modes
  • A changelog entry exists and is marked appropriately

@andreadelrio
Copy link
Contributor Author

@myasonik this is my first pass at trying to cover #2335. Let me know what you think.

@myasonik
Copy link
Contributor

From an a11y perspective, I think this is all that's needed and is good to go 👍 🎉

@andreadelrio andreadelrio marked this pull request as ready for review January 30, 2020 19:20
Copy link
Contributor

@thompsongl thompsongl left a comment

Choose a reason for hiding this comment

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

One suggestion, otherwise LGTM!

src/components/basic_table/basic_table.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

@myasonik myasonik left a comment

Choose a reason for hiding this comment

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

VoiceOver output is correct 🎉

@andreadelrio
Copy link
Contributor Author

After checking with @myasonik this also covers #2337 as CJ had suggested here

The screen reader does "[row x out of total] [name of row]*, Select this row"
*identifying column used in rowHeader
select_this_row

@andreadelrio andreadelrio merged commit 74ebe2b into elastic:master Jan 31, 2020
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.

Improve accessibility of EuiTableRowCell
3 participants