[Dashboard] [Controls] Make floating actions keyboard accessible #152155

Heenawter · 2023-02-24T22:48:31Z

Summary

This PR adds the floating actions back into the regular HTML tree by removing the EuiPortal that surrounded them, thus making them keyboard accessible via some added CSS.

In order to do this, however, there were a few changes that had to be made to the overall Dashboard HTML structure. Previously, as part of relocating the Dashboard scrollbar, the scrollable section of the app was moved to the Dashboard viewport, like so:

Screen.Recording.2023-03-02.at.10.54.43.AM.mov

While this had a lot of visual appeal, because of the structure of the HTML tree, the floating actions had to be moved to an EuiPortal as part of this change so that they would continue to float above the top navigation bar rather than clipping behind it alongside the other contents of the viewport - this made it impossible to add native keyboard accessibility since they were removed from the natural HTML structure.

Unfortunately, by removing the EuiPortal in order to allow for keyboard accessibility, this meant that the scrollable section could no longer be constrained to the viewport - this is because the z-index of child of a given scrollable div is always relative to its parent, which means that the floating actions would clip behind the top nav bar regardless of how high you set their z-index:

Therefore, in order to avoid this clipping, the scrollable section had to remain at the top of the app, like so:

Screen.Recording.2023-03-02.at.10.56.25.AM.mov

In order to keep the benefit of the top query bar remaining in place, the top nav bar was added to a fixed position div that floats above the contents of the viewport as the user scrolls - this ensures that the floating actions, which are now also positioned via a fixed container, can still float above the nav bar while remaining part of the natural order of the HTML tree.

As a follow up, we should also address #152609.

Checklist

Unit or functional tests were updated or added to match the most common scenarios
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

For maintainers

This was checked for breaking API changes and was labeled appropriately

Heenawter · 2023-03-02T18:44:02Z

examples/portable_dashboards_example/public/static_by_reference_example.tsx

        // constrained to the container - so, the dashboard is rendered with a vertical scrollbar
        css={css`
          height: 600px;
+          overflow-y: auto;


Because the Dashboard viewport is no longer constrained to the height of its parent container, we need the container (in this case, the EuiPanel) to be responsible for handling the overflow.

Heenawter · 2023-03-02T18:49:06Z

test/functional/services/dashboard/panel_actions.ts

+    if (parent) {
+      await parent.scrollIntoViewIfNecessary(DASHBOARD_TOP_OFFSET);
+      await this.browser.getActions().move({ x: 0, y: 0, origin: parent._webElement }).perform();
+    } else {
+      await this.testSubjects.moveMouseTo('dashboardPanelTitle');
+    }


The web_element_wrapper service does not accept a topOffset for their moveMouseTo function - this was causing issues because, at the top of this function, they call scrollIntoViewIfNecessary without taking in to account the necessary offset. This was causing the top of the panels to be overlapped by the sticky top nav bar, even if they were already in view before this function was called.

Rather than requesting a review from the QA team to make their version of moveMouseTo accept a top offset for the scroll, I decided to just recreate the actions here instead 🤷

elasticmachine · 2023-03-02T18:50:04Z

Pinging @elastic/kibana-accessibility (Project:Accessibility)

elasticmachine · 2023-03-02T18:50:06Z

Pinging @elastic/kibana-presentation (Team:Presentation)

ThomThomson

Looked through the code and left a few comments + one nit.

Tested this locally and everything works great!

Just out of curiosity, is there any way we could swap the scrollbar design for the EUI scrollbar as a later enhancement?

src/plugins/controls/public/control_group/component/control_frame_component.tsx

src/plugins/dashboard/public/dashboard_app/top_nav/_dashboard_top_nav.scss

src/plugins/dashboard/public/dashboard_app/top_nav/dashboard_top_nav.tsx

src/plugins/presentation_util/public/components/floating_actions/floating_actions.tsx

Heenawter · 2023-03-06T16:56:26Z

Just out of curiosity, is there any way we could swap the scrollbar design for the EUI scrollbar as a later enhancement?

@ThomThomson The scrollbar now belongs to the body of the HTML tree, so I don't think we can make any changes to the style of the scrollbar without that applying to all apps unfortunately.

ThomThomson · 2023-03-06T17:00:29Z

That makes sense, thank you for the clarification!

kibana-ci · 2023-03-06T18:30:24Z

💚 Build Succeeded

Buildkite Build
Commit: 5acb477

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`dashboard`	377	382	+5
`presentationUtil`	149	154	+5
total			+10

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`controls`	179.2KB	179.4KB	+195.0B
`dashboard`	385.5KB	386.5KB	+1018.0B
`presentationUtil`	146.1KB	129.0KB	-17.1KB
total			-15.9KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`presentationUtil`	31.6KB	35.2KB	+3.7KB

Unknown metric groups

ESLint disabled line counts

id	before	after	diff
`securitySolution`	428	430	+2

Total ESLint disabled count

id	before	after	diff
`securitySolution`	506	508	+2

History

💚 Build #112110 succeeded e48d610
💚 Build #111779 succeeded d6cb83a
💚 Build #111752 succeeded efaa329
💔 Build #111567 failed 7735e0a
💔 Build #111554 failed 4f475db
💚 Build #111518 succeeded 666c5ea

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

andreadelrio

LGTM, nice improvement.

…stic#152155) Closes elastic#135458 Unblocks elastic#151233 ## Summary This PR adds the floating actions back into the regular HTML tree by removing the `EuiPortal` that surrounded them, thus making them keyboard accessible via some added CSS. ![Mar-02-2023 11-54-13](https://user-images.githubusercontent.com/8698078/222524586-8051b8e5-fe1e-48b2-bd83-30a90f9b3417.gif) In order to do this, however, there were a few changes that had to be made to the overall Dashboard HTML structure. Previously, as part of [relocating the Dashboard scrollbar](elastic#145628), the scrollable section of the app was moved to the Dashboard viewport, like so: https://user-images.githubusercontent.com/8698078/222511861-8707917c-9edc-4292-a182-58924bb00c8a.mov While this had a lot of visual appeal, because of the structure of the HTML tree, the floating actions had to be moved to an `EuiPortal` as part of this change so that they would continue to float above the top navigation bar rather than clipping behind it alongside the other contents of the viewport - this made it impossible to add native keyboard accessibility since they were removed from the natural HTML structure. Unfortunately, by removing the `EuiPortal` in order to allow for keyboard accessibility, this meant that the scrollable section could **no longer** be constrained to the viewport - this is because the `z-index` of child of a given scrollable `div` is **always relative** to its parent, which means that the floating actions would clip behind the top nav bar regardless of how high you set their `z-index`: <img src="https://user-images.githubusercontent.com/8698078/222518354-80f1df75-69e5-4433-a256-d0b7dc57cd97.gif"/> Therefore, in order to avoid this clipping, the scrollable section had to remain at the top of the app, like so: https://user-images.githubusercontent.com/8698078/222512203-60a88fc5-dd68-47ba-aeab-2425afc60a67.mov In order to keep the benefit of the top query bar remaining in place, the top nav bar was added to a **fixed position** `div` that floats above the contents of the viewport as the user scrolls - this ensures that the floating actions, which are now also positioned via a `fixed` container, can still float above the nav bar while remaining part of the natural order of the HTML tree. As a follow up, we should also address elastic#152609. ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [x] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US)) - [x] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [x] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

…2155) Closes #135458 Unblocks #151233 ## Summary This PR adds the floating actions back into the regular HTML tree by removing the `EuiPortal` that surrounded them, thus making them keyboard accessible via some added CSS. ![Mar-02-2023 11-54-13](https://user-images.githubusercontent.com/8698078/222524586-8051b8e5-fe1e-48b2-bd83-30a90f9b3417.gif) In order to do this, however, there were a few changes that had to be made to the overall Dashboard HTML structure. Previously, as part of [relocating the Dashboard scrollbar](#145628), the scrollable section of the app was moved to the Dashboard viewport, like so: https://user-images.githubusercontent.com/8698078/222511861-8707917c-9edc-4292-a182-58924bb00c8a.mov While this had a lot of visual appeal, because of the structure of the HTML tree, the floating actions had to be moved to an `EuiPortal` as part of this change so that they would continue to float above the top navigation bar rather than clipping behind it alongside the other contents of the viewport - this made it impossible to add native keyboard accessibility since they were removed from the natural HTML structure. Unfortunately, by removing the `EuiPortal` in order to allow for keyboard accessibility, this meant that the scrollable section could **no longer** be constrained to the viewport - this is because the `z-index` of child of a given scrollable `div` is **always relative** to its parent, which means that the floating actions would clip behind the top nav bar regardless of how high you set their `z-index`: <img src="https://user-images.githubusercontent.com/8698078/222518354-80f1df75-69e5-4433-a256-d0b7dc57cd97.gif"/> Therefore, in order to avoid this clipping, the scrollable section had to remain at the top of the app, like so: https://user-images.githubusercontent.com/8698078/222512203-60a88fc5-dd68-47ba-aeab-2425afc60a67.mov In order to keep the benefit of the top query bar remaining in place, the top nav bar was added to a **fixed position** `div` that floats above the contents of the viewport as the user scrolls - this ensures that the floating actions, which are now also positioned via a `fixed` container, can still float above the nav bar while remaining part of the natural order of the HTML tree. As a follow up, we should also address #152609. ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [x] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US)) - [x] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [x] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

Heenawter force-pushed the fix-floating-actions-a11y_2023-02-23 branch 9 times, most recently from 993d1f0 to b2529d8 Compare February 28, 2023 19:06

Heenawter added the Project:Accessibility label Feb 28, 2023

Heenawter force-pushed the fix-floating-actions-a11y_2023-02-23 branch 4 times, most recently from b607541 to b430bec Compare March 1, 2023 20:36

Heenawter added 6 commits March 1, 2023 13:36

Made the toolbar sticky instead of external

0d45be9

Fix maximized panel

810f4b5

Dynamically determine padding

40e96a9

Make floating actions visible on focus

83f4a57

Update screenshots

c471148

Add fixed header height

8209c65

Heenawter force-pushed the fix-floating-actions-a11y_2023-02-23 branch from b430bec to 17d0107 Compare March 1, 2023 20:37

Fix options list tests + remove fixed header height

666c5ea

Heenawter force-pushed the fix-floating-actions-a11y_2023-02-23 branch from 17d0107 to 666c5ea Compare March 1, 2023 20:46

Heenawter added 2 commits March 1, 2023 15:03

Fix example plugin

13fbc7e

Clean up code + styling

387e59e

Heenawter force-pushed the fix-floating-actions-a11y_2023-02-23 branch 2 times, most recently from 8b2995f to 7735e0a Compare March 2, 2023 00:07

Heenawter added 2 commits March 2, 2023 10:05

Move functional test fix from web_element_wrapper to panel_actions

632e057

Clean up top nav bar code

d6cb83a

Heenawter force-pushed the fix-floating-actions-a11y_2023-02-23 branch from c44397d to d6cb83a Compare March 2, 2023 17:06

Heenawter commented Mar 2, 2023

View reviewed changes

Heenawter marked this pull request as ready for review March 2, 2023 18:50

Heenawter requested review from a team as code owners March 2, 2023 18:50

Merge branch 'main' into fix-floating-actions-a11y_2023-02-23

e48d610

ThomThomson approved these changes Mar 6, 2023

View reviewed changes

Remove duplicated state

5acb477

andreadelrio approved these changes Mar 6, 2023

View reviewed changes

Heenawter merged commit 8fec139 into elastic:main Mar 6, 2023

kibanamachine added the v8.8.0 label Mar 6, 2023

ThomThomson mentioned this pull request May 8, 2023

[DOCS] Adds 8.8.0 release notes #155951

Merged

wayneseymour mentioned this pull request Apr 29, 2024

[FAILS ON MKI] Serverless Search Functional Tests.x-pack/test_serverless/functional/test_suites/search/dashboards/build_dashboard.ts #181956

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Dashboard] [Controls] Make floating actions keyboard accessible #152155

[Dashboard] [Controls] Make floating actions keyboard accessible #152155

Heenawter commented Feb 24, 2023 •

edited

Heenawter Mar 2, 2023

Heenawter Mar 2, 2023 •

edited

elasticmachine commented Mar 2, 2023

elasticmachine commented Mar 2, 2023

ThomThomson left a comment

Heenawter commented Mar 6, 2023

ThomThomson commented Mar 6, 2023

kibana-ci commented Mar 6, 2023

ESLint disabled line counts

Total ESLint disabled count

andreadelrio left a comment

[Dashboard] [Controls] Make floating actions keyboard accessible #152155

[Dashboard] [Controls] Make floating actions keyboard accessible #152155

Conversation

Heenawter commented Feb 24, 2023 • edited

Summary

Checklist

For maintainers

Heenawter Mar 2, 2023

Choose a reason for hiding this comment

Heenawter Mar 2, 2023 • edited

Choose a reason for hiding this comment

elasticmachine commented Mar 2, 2023

elasticmachine commented Mar 2, 2023

ThomThomson left a comment

Choose a reason for hiding this comment

Heenawter commented Mar 6, 2023

ThomThomson commented Mar 6, 2023

kibana-ci commented Mar 6, 2023

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Page load bundle

ESLint disabled line counts

Total ESLint disabled count

History

andreadelrio left a comment

Choose a reason for hiding this comment

Heenawter commented Feb 24, 2023 •

edited

Heenawter Mar 2, 2023 •

edited