search

elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.76k stars 8.17k forks source link

Enterprise search needs a11y tests #79359

Open bhavyarm opened 4 years ago

bhavyarm commented 4 years ago

Please add a11y tests to Enterprise search UI in Kibana - for overview page, app search and workplace search. Here are the documents which explain a11y testing - https://www.elastic.co/guide/en/kibana/master/development-tests.html#development-accessibility-tests

Here is the meta issue where you can find other test PRs: https://github.com/elastic/kibana/issues/51456

Please get in touch with @myasonik or @bhavyarm if you have questions about these tests.

elasticmachine commented 4 years ago

Pinging @elastic/app-search-frontend (Team:AppSearch)

cee-chen commented 4 years ago

Thanks @bhavyarm! We can definitely add accessibility tests for the following plugins/pages:

... But unfortunately, I'm not sure we can (easily) add accessibility tests deeper into the products (which at the moment isn't a whole lot, so it's not a huge deal, but might be later on).

The problem is that our plugin requires Enterprise Search to run, and as far as we know (or were intending on implementing), Kibana's CI pipeline can't spin it up as a sidecar service.

I'm not too sure how in-depth these aXe reports/screenshots are compared to the E2E functional tests - if they're straightforward, it's possible we could somehow feed mock API endpoint data to the plugin, but that's still a pretty high lift for us right now when our focus is on fully migrating 2 products to Kibana by 8.0.

If basic a11y tests for just the above 3 pages (that don't require Enterprise Search to be configured/running) are OK for now, I can definitely look at implementing them, and circle back later after our full migration to more in-depth tests.

myasonik commented 3 years ago

If basic a11y tests for just the above 3 pages (that don't require Enterprise Search to be configured/running) are OK for now, I can definitely look at implementing them, and circle back later after our full migration to more in-depth tests.

With the constraints you laid out, this sounds like a great place to start!

cee-chen commented 3 years ago

Awesome!! Thanks so much Michail. Accessibility requirements are definitely important to me personally, but as always it's a tough balancing act w/ our deadlines as well as figuring out what tooling makes the most sense long-term for running our functional/E2E tests. For what it's worth I'm trying to manually audit as much as code as I can accessibility-wise as we migrate it to Kibana.

I'll create a ticket in our migration epic for this and shoot to have the 3 pages listed above in Kibana CI by 7.11

cee-chen commented 3 years ago

👋 Apologies this took so dang long y'all :) I have a11y tests added for the scope we mentioned above (i.e., a very small portion of the actual full app): https://github.com/elastic/kibana/pull/106765

I'm honestly not super sure if that counts as closing/completing this ticket or if you would consider this completed only if the full app gets tested. This might be complex as we were planning on writing our E2E tests in Cypress, and not necessarily Kibana's FTR. Would using an equivalent a11y plugin in Cypress pass your requirements for having full a11y tests?

myasonik commented 3 years ago

No worries! Your work is awesome!

Deque, the makers of axe, have written about setting up axe with cypress so you should be in good company.

There aren't strict requirements on how to test so whatever works for y'all will be good. This is ultimately your ticket so feel free to track however is best for your team.

elasticmachine commented 3 years ago

Pinging @elastic/workplace-search-frontend (Team:WorkplaceSearch)

byronhulcher commented 3 years ago

I agree if we could test for this in Cypress it would make a lot more sense (since we'll have to solve the "how do we run this in a test harness" problem for Cypress).

I'm honestly not super sure if that counts as closing/completing this ticket

I like what you did in https://github.com/elastic/kibana/pull/106765 @constancecchen. I think as much as much as its a bummer we should consider leaving this open until we implement this in Cypress (or solve the mock API problem in FTR). I think having a11y tests on our light manual testing / critical path pages (and components?) would be a good compromise requirement? We could prioritize the remaining views/components after that.

cee-chen commented 3 years ago

Love it. Huge ++!

cee-chen commented 3 years ago

👋 Hey @elastic/app-search-frontend & @elastic/workplace-search-backend!

https://github.com/elastic/kibana/pull/108465 (once merged in) should set up the ability to automate cypress-axe checks on any views you need. I would suggest setting up basic "visit route, run checkA11y()" type tests for both AS & WS and then optionally get more complex from there if needed (e.g., for pages with flyouts - open the flyouts and then run checkA11y() again, scoped to the flyout).

It hopefully shouldn't be super high-effort, I imagine this wouldn't take more than a few days for a single dev for each product (given that there aren't a bunch of existing failures to fix, knock on wood).

All the best!