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

Fix accessibility issues with saved object finder #13152

Merged

Conversation

Projects
None yet
4 participants
@stacey-gammon
Copy link
Contributor

commented Jul 27, 2017

and use new kui styles

Fixes #12484
Fixes #12485
Fixes #12483

screen shot 2017-07-27 at 11 24 47 am

@cjcenizal
Copy link
Contributor

left a comment

I love that you swapped in the kuiTabs component -- thanks for doing that! I have just one small suggestion.

</li>
</ul>
<div class="kuiTabs">
<div

This comment has been minimized.

Copy link
@cjcenizal

cjcenizal Jul 27, 2017

Contributor

These can be <button> elements, and then we won't need kbn-accessible-click any more. That directive is meant to be used as a last resort, if you can't use a <button> or <a> with an href.

@cjcenizal

This comment has been minimized.

Copy link
Contributor

commented Jul 27, 2017

Also can we leave #12205 open? I think these will need to be long-lived issues to help us track overall progress and write up our 508 accessibility report.

@trevan

This comment has been minimized.

Copy link
Contributor

commented Jul 28, 2017

I don't think kuiTabs are darkThemed yet so this will look wrong on a dark theme dashboard.

@cjcenizal

This comment has been minimized.

Copy link
Contributor

commented Jul 28, 2017

Oops @trevan is right. I guess we'll have to forgo the tabs for now, unless you want to update the UI Framework @stacey-gammon.

@stacey-gammon

This comment has been minimized.

Copy link
Contributor Author

commented Jul 28, 2017

I'm probably going about this all wrong, but some experimentation yields this for a dark theme variation:

screen shot 2017-07-28 at 5 07 26 pm

screen shot 2017-07-28 at 5 07 32 pm

@cjcenizal
Copy link
Contributor

left a comment

🛠 It's so cool to see this component get updated. Thanks for doing this. Just had two suggestions!

$tabColor--darkTheme: #cecece;
$tabBackgroundColor--darkTheme: #333333;
$tabHoverBackgroundColor--darkTheme: #777777;
$tabBorderColor--darkTheme: #777777;

This comment has been minimized.

Copy link
@cjcenizal

cjcenizal Jul 28, 2017

Contributor

This is fantastic! I'd just suggest looking for global color variables (in https://github.com/elastic/kibana/blob/master/ui_framework/components/_variables.scss#L47), to which you can assign these variables. For example:

$tabColor--darkTheme: $globalTextColor--darkTheme;
$tabBackgroundColor--darkTheme: $globalBackgroundColor--darkTheme;
// etc...

This way all of the components will derive the same color palette from a shared source of truth. If there are any colors which don't exist in the _variables.scss file, then feel free to just assign the hex value to a component-specific variable in this file.

@@ -23,6 +27,11 @@
border-radius: 0; /* 1 */
margin-bottom: -1px; /* 3 */

@include darkTheme {
background-color: $tabBackgroundColor--darkTheme;
border-color: $tabBorderColor--darkTheme;

This comment has been minimized.

Copy link
@cjcenizal

cjcenizal Jul 28, 2017

Contributor

I think we'll also need to add a color property here to make it match the link's dark theme style (https://github.com/elastic/kibana/blob/master/ui_framework/components/_mixins.scss#L25).

color: $globalLinkColor--darkTheme;

Similar changes will need to be made to the hover and focus states.

This comment has been minimized.

Copy link
@stacey-gammon

stacey-gammon Aug 1, 2017

Author Contributor

I added colors for the hover states, but the other states I left as is to follow suit with the light theme, which doesn't treat those tabs when they are active as links.

How it looks now:

screen shot 2017-07-31 at 12 23 14 pm

This comment has been minimized.

Copy link
@cjcenizal

cjcenizal Aug 1, 2017

Contributor

Oh OK cool, I see. I took a look in the browser and it looks like the text color has poor color contrast against the background:

image

I think we should address this color contrast issue. How about using a different variable -- can we add color: $tabColor--darkTheme;? This will get it to look like this:

image

@stacey-gammon

This comment has been minimized.

Copy link
Contributor Author

commented Jul 31, 2017

jenkins, test this

@cjcenizal
Copy link
Contributor

left a comment

Had one small suggestion. Also, can we add an example for the Tabs dark theme to the documentation site?

<GuideDemo isDarkTheme={true}>
  <Tabs />
</GuideDemo>
@@ -23,6 +27,11 @@
border-radius: 0; /* 1 */
margin-bottom: -1px; /* 3 */

@include darkTheme {
background-color: $tabBackgroundColor--darkTheme;
border-color: $tabBorderColor--darkTheme;

This comment has been minimized.

Copy link
@cjcenizal

cjcenizal Aug 1, 2017

Contributor

Oh OK cool, I see. I took a look in the browser and it looks like the text color has poor color contrast against the background:

image

I think we should address this color contrast issue. How about using a different variable -- can we add color: $tabColor--darkTheme;? This will get it to look like this:

image

stacey-gammon added some commits Aug 2, 2017

@stacey-gammon

This comment has been minimized.

Copy link
Contributor Author

commented Aug 2, 2017

Failed on:

03:35:17.578        │ debg  existsByDisplayedByCssSelector [data-test-subj~="dashboardLandingPage"]
03:35:18.626        │ debg  clickDashboardBreadcrumbLink
03:35:18.691        │ debg  TestSubjects.find(searchFilter)
03:35:18.692        │ debg  in displayedByCssSelector: [data-test-subj~="searchFilter"]
03:35:28.745        │ debg  --- tryForTime failure: An element could not be located on the page using the given search parameters.
03:35:39.287        │ debg  --- tryForTime failed again with the same message  ...
03:35:49.856        │ debg  --- tryForTime failed again with the same message  ...
03:36:00.425        │ debg  --- tryForTime failed again with the same message  ...
03:36:00.992        │ debg  --- tryForTime failure: tryForTime timeout: NoSuchElement: An element could not be located on the page using the given search parameters.
03:36:00.995        │           at /var/lib/jenkins/workspace/elastic+kibana+pull-request+multijob-selenium/node_modules/leadfoot/lib/findDisplayed.js:37:21
03:36:00.995        │           at /var/lib/jenkins/workspace/elastic+kibana+pull-request+multijob-selenium/node_modules/dojo/Promise.js:156:41
03:36:00.995        │           at run (/var/lib/jenkins/workspace/elastic+kibana+pull-request+multijob-selenium/node_modules/dojo/Promise.js:51:33)
03:36:00.995        │           at /var/lib/jenkins/workspace/elastic+kibana+pull-request+multijob-selenium/node_modules/dojo/nextTick.js:35:17
03:36:00.995        │           at _combinedTickCallback (internal/process/next_tick.js:73:7)
03:36:00.995        │           at process._tickDomainCallback (internal/process/next_tick.js:128:9)
03:36:00.995        │           at Command.findDisplayed (/var/lib/jenkins/workspace/elastic+kibana+pull-request+multijob-selenium/node_modules/leadfoot/Command.js:23:10)
....
(/var/lib/jenkins/workspace/elastic+kibana+pull-request+multijob-selenium/node_modules/bluebird/js/main/async.js:15:14)
03:36:01.002        │           at runCallback (timers.js:672:20)
03:36:01.003        │           at tryOnImmediate (timers.js:645:5)
03:36:01.003        │           at processImmediate [as _immediateCallback] (timers.js:617:5)
03:36:01.498        │ debg  --- tryForTime failure: tryForTime timeout: Error: tryForTime timeout: NoSuchElement: An element could not be located on the page using the given search parameters.
03:36:01.500        │           at /var/lib/jenkins/workspace/elastic+kibana+pull-request+multijob-selenium/node_modules/leadfoot/lib/findDisplayed.js:37:21
03:36:01.500        │           at /var/lib/jenkins/workspace/elastic+kibana+pull-request+multijob-selenium/node_modules/dojo/Promise.js:156:41
......
03:36:01.507        │           at tryOnImmediate (timers.js:645:5)
03:36:01.507        │           at processImmediate [as _immediateCallback] (timers.js:617:5)
03:36:02.004        │ debg  Taking screenshot "/var/lib/jenkins/workspace/elastic+kibana+pull-request+multijob-selenium/test/functional/screenshots/failure/dashboard app dashboard clone clone warns on duplicate name.png"
03:36:02.091      └- ✖ fail: "dashboard app dashboard clone clone warns on duplicate name"
03:36:02.093      │        tryForTime timeout: Error: tryForTime timeout: Error: tryForTime timeout: NoSuchElement: An element could not be located on the page using the given search parameters.

Hope this isn't an unstable test, those were supposed to be fixed. :(

If it passes a second time, I'll file a bug for unstable test. Since it seems to have failed on the merge commit and not the prior commit, that's what I suspect.

jenkins, test this

@stacey-gammon

This comment has been minimized.

Copy link
Contributor Author

commented Aug 3, 2017

Should be ready to go for a final look @cjcenizal. Test failure was flaky and I have a separate PR out that should address it.

@cjcenizal
Copy link
Contributor

left a comment

Fantastic! 🌋

@stacey-gammon stacey-gammon merged commit ab8668a into elastic:master Aug 7, 2017

2 checks passed

CLA Commit author has signed the CLA
Details
kibana-ci Build finished.
Details

stacey-gammon added a commit to stacey-gammon/kibana that referenced this pull request Aug 7, 2017

Fix accessibility issues with saved object finder (elastic#13152)
* Fix accessibility issues with saved object finder

and use new kui styles

* Dark theme-icy kuiTabs

* Refer to existing dark theme color variables.  Use dark theme hover link color.

* use button instead of div element so no need for kbn-accessible-click

* Add dark theme tab variety to ui framework site, lighten color of background tabs

stacey-gammon added a commit to stacey-gammon/kibana that referenced this pull request Aug 7, 2017

Fix accessibility issues with saved object finder (elastic#13152)
* Fix accessibility issues with saved object finder

and use new kui styles

* Dark theme-icy kuiTabs

* Refer to existing dark theme color variables.  Use dark theme hover link color.

* use button instead of div element so no need for kbn-accessible-click

* Add dark theme tab variety to ui framework site, lighten color of background tabs
@stacey-gammon

This comment has been minimized.

Copy link
Contributor Author

commented Aug 7, 2017

Backports:
6.x: #13371
6.0: #13372

stacey-gammon added a commit that referenced this pull request Aug 8, 2017

Fix accessibility issues with saved object finder (#13152) (#13371)
* Fix accessibility issues with saved object finder

and use new kui styles

* Dark theme-icy kuiTabs

* Refer to existing dark theme color variables.  Use dark theme hover link color.

* use button instead of div element so no need for kbn-accessible-click

* Add dark theme tab variety to ui framework site, lighten color of background tabs

stacey-gammon added a commit that referenced this pull request Aug 8, 2017

Fix accessibility issues with saved object finder (#13152) (#13372)
* Fix accessibility issues with saved object finder

and use new kui styles

* Dark theme-icy kuiTabs

* Refer to existing dark theme color variables.  Use dark theme hover link color.

* use button instead of div element so no need for kbn-accessible-click

* Add dark theme tab variety to ui framework site, lighten color of background tabs

@stacey-gammon stacey-gammon deleted the stacey-gammon:fix/accessibility-object-finder branch Oct 24, 2017

chrisronline added a commit to chrisronline/kibana that referenced this pull request Nov 20, 2017

Fix accessibility issues with saved object finder (elastic#13152)
* Fix accessibility issues with saved object finder

and use new kui styles

* Dark theme-icy kuiTabs

* Refer to existing dark theme color variables.  Use dark theme hover link color.

* use button instead of div element so no need for kbn-accessible-click

* Add dark theme tab variety to ui framework site, lighten color of background tabs

chrisronline added a commit to chrisronline/kibana that referenced this pull request Dec 1, 2017

Fix accessibility issues with saved object finder (elastic#13152)
* Fix accessibility issues with saved object finder

and use new kui styles

* Dark theme-icy kuiTabs

* Refer to existing dark theme color variables.  Use dark theme hover link color.

* use button instead of div element so no need for kbn-accessible-click

* Add dark theme tab variety to ui framework site, lighten color of background tabs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.