-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Improve Visualize screen-reader accessibility. #11599
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -86,11 +86,12 @@ | |
Custom Ranges | ||
</span> | ||
</div> | ||
<input aria-label="enable" | ||
ng-model="vis.params.setColorRange" | ||
type="checkbox" | ||
class="kuiSideBarSectionTitle__action" | ||
ng-click="toggleColorRangeSection(true)" | ||
<input | ||
aria-label="Enable custom ranges" | ||
ng-model="vis.params.setColorRange" | ||
type="checkbox" | ||
class="kuiSideBarSectionTitle__action" | ||
ng-click="toggleColorRangeSection(true)" | ||
> | ||
</div> | ||
|
||
|
@@ -172,10 +173,11 @@ | |
Show Labels | ||
</span> | ||
</div> | ||
<input aria-label="enable" | ||
ng-model="valueAxis.labels.show" | ||
type="checkbox" | ||
class="kuiSideBarSectionTitle__action" | ||
<input | ||
aria-label="Show labels" | ||
ng-model="valueAxis.labels.show" | ||
type="checkbox" | ||
class="kuiSideBarSectionTitle__action" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No changes here, just formatting. |
||
> | ||
</div> | ||
<div ng-if="valueAxis.labels.show" ng-show="showLabels" class="kuiSideBarCollapsibleSection"> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,9 +5,10 @@ | |
</div> | ||
|
||
<button | ||
aria-label="Add value axis" | ||
data-test-subj="visualizeAddYAxisButton" | ||
aria-label="Add Y axis" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you confirm that we should use the term "Y axis" here instead of "value axis"? |
||
ng-click="addValueAxis()" | ||
tooltip="Add value axis" | ||
tooltip="Add Y axis" | ||
tooltip-append-to-body="true" | ||
type="button" | ||
class="fa fa-plus kuiSideBarSectionTitle__action" | ||
|
@@ -37,9 +38,9 @@ | |
|
||
<button | ||
ng-hide="vis.params.valueAxes.length === 1" | ||
aria-label="Remove value axis" | ||
aria-label="Remove Y axis" | ||
ng-click="removeValueAxis(axis)" | ||
tooltip="Remove value axis" | ||
tooltip="Remove Y axis" | ||
tooltip-append-to-body="true" | ||
type="button" | ||
class="fa fa-remove kuiSideBarCollapsibleTitle__action" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,7 +6,12 @@ | |
novalidate | ||
><!-- see http://goo.gl/9kgz5w --> | ||
|
||
<div css-truncate title="{{indexPattern.id}}" ng-if="vis.type.requiresSearch" class="index-pattern"> | ||
<div | ||
css-truncate | ||
aria-label="{{:: 'Index pattern: ' + indexPattern.id}}" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should use aria-label here instead of title. Because aria-label overrides any text content, we also need to include the index pattern ID here. |
||
ng-if="vis.type.requiresSearch" | ||
class="index-pattern" | ||
> | ||
{{ indexPattern.id }} | ||
</div> | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,15 +27,21 @@ class IconOption extends Component { | |
|
||
render() { | ||
const icon = this.props.option.value; | ||
const title = this.props.option.label; | ||
const label = this.props.option.label; | ||
return ( | ||
<div className={this.props.className} | ||
onMouseEnter={this.handleMouseEnter} | ||
onMouseDown={this.handleMouseDown} | ||
onMouseMove={this.handleMouseMove} | ||
title={title}> | ||
<span className="Select-value-label"> | ||
<i className={`vis_editor__icon_select-option fa ${icon}`}></i> | ||
> | ||
<span | ||
className="Select-value-label" | ||
aria-label={`${label} icon`} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should use aria-label instead of title. And to provide context, we'll also append the word "icon". |
||
> | ||
<span | ||
className={`vis_editor__icon_select-option kuiIcon ${icon}`} | ||
aria-hidden="true" | ||
></span> | ||
{ this.props.children } | ||
</span> | ||
</div> | ||
|
@@ -60,9 +66,12 @@ function IconValue(props) { | |
const icon = props.value && props.value.value; | ||
const label = props.value && props.value.label; | ||
return ( | ||
<div className="Select-value" title={label}> | ||
<span className="Select-value-label"> | ||
<i className={`vis_editor__icon_select-value fa ${icon}`}></i> | ||
<div className="Select-value"> | ||
<span | ||
className="Select-value-label" | ||
aria-label={`${label} icon`} | ||
> | ||
<span className={`vis_editor__icon_select-value kuiIcon ${icon}`}></span> | ||
{ props.children } | ||
</span> | ||
</div> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No changes here, just formatting.