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

[ML] Accessibility fix for structural markup on table rows #55075

Merged

Conversation

peteharverson
Copy link
Contributor

@peteharverson peteharverson commented Jan 16, 2020

Summary

Fixes #52373, adding structural markup with scope="row" to identify each table row for assistive technology.

Checklist

For maintainers

Fixes #52373

@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

Copy link
Contributor

@walterra walterra left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@jgowdyelastic jgowdyelastic left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@alvarezmelissa87 alvarezmelissa87 left a comment

Choose a reason for hiding this comment

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

LGTM ⚡️

@@ -28,6 +28,7 @@ export interface FieldDataColumnType<T> {
render?: RenderFunc;
footer?: string | ReactElement | FooterFunc;
textOnly?: boolean;
scope?: 'col' | 'row' | 'colgroup' | 'rowgroup';
Copy link
Contributor

Choose a reason for hiding this comment

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

is it possible to import this type from eui?

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks so, it is TableHeaderCellScope

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Decided to copy those scope values locally for now. We plan to switch over to using the types which are now available in EUI, hopefully for 7.7.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@peteharverson peteharverson merged commit d740ec3 into elastic:master Jan 17, 2020
@peteharverson peteharverson deleted the ml-table-accessibility-row-scope branch January 17, 2020 13:48
peteharverson added a commit to peteharverson/kibana that referenced this pull request Jan 17, 2020
…5075)

* [ML] Accessibility fix for structural markup on table rows

* [ML] Fix type error on Transforms table cell for scope property
peteharverson added a commit to peteharverson/kibana that referenced this pull request Jan 17, 2020
…5075)

* [ML] Accessibility fix for structural markup on table rows

* [ML] Fix type error on Transforms table cell for scope property
jkelastic pushed a commit to jkelastic/kibana that referenced this pull request Jan 17, 2020
…5075)

* [ML] Accessibility fix for structural markup on table rows

* [ML] Fix type error on Transforms table cell for scope property
peteharverson added a commit that referenced this pull request Jan 17, 2020
…55181)

* [ML] Accessibility fix for structural markup on table rows

* [ML] Fix type error on Transforms table cell for scope property
peteharverson added a commit that referenced this pull request Jan 17, 2020
…55185)

* [ML] Accessibility fix for structural markup on table rows

* [ML] Fix type error on Transforms table cell for scope property
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jan 20, 2020
* upstream/master: (83 commits)
  [Reporting] Fix map tiles not loading by using Chrome's Remote Protocol (elastic#55137)
  [Data Plugin] combine autocomplete provider and suggestions provider (elastic#54451)
  resolves elastic#53038 - remove references to specific license levels (elastic#53858)
  highlighting rules should still know about url parts when in sql state (elastic#55200)
  [Metric] convert mocha tests to jest (elastic#54054)
  [skip-ci] Update vector styling docs for 7.6 UI changes and new features (elastic#55087)
  Fix enable API to schedule task after alert is updated (elastic#55095)
  chore(NA): add 7.6 branch to the list of backport branches (elastic#54998)
  Convert tests to jest in vis_type_timeseries/public & common folders (elastic#55023)
  [ML] Accessibility fix for structural markup on table rows (elastic#55075)
  [Mappings editor] include/exclude fields only support custom options (elastic#54949)
  [Vis] Move Timelion Vis to vis_type_timelion (elastic#52069)
  Deprecate `chrome.navlinks.update` and add documentation (elastic#54893)
  [ML] Single Metric Viewer: Fix time bounds with custom strings. (elastic#55045)
  [Vis: Default editor] EUIficate and Reactify the sidebar (elastic#49864)
  [Mappings editor] Fix cannot set boolean value for "null_value" param (elastic#55015)
  [SIEM] Adds support for apm-* to the network map (elastic#54876)
  [Reporting] Define shims of legacy dependencies (elastic#54082)
  Resolver is overflow: hidden to prevent obscured elements from showing up (elastic#55076)
  Upgraded EUI to 18.2.1 (elastic#55090)
  ...
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.

[ML] (Accessibility) - Machine Learning - Anomaly Detection / Job Management - Table -
7 participants