From 33d03837795cea0bd80bf2af01e77f27cdf97b9c Mon Sep 17 00:00:00 2001 From: John Cowen Date: Tue, 9 Mar 2021 09:30:01 +0000 Subject: [PATCH] ui: a11y modals (#9819) This PR uses the excellent a11y-dialog to implement our modal functionality across the UI. This package covers all our a11y needs - overlay click and ESC to close, controlling aria-* attributes, focus trap and restore. It's also very small (1.6kb) and has good DOM and JS APIs and also seems to be widely used and well tested. There is one downside to using this, and that is: We made use of a very handy characteristic of the relationship between HTML labels and inputs in order to implement our modals previously. Adding a for="id" attribute to a label meant you can control an from anywhere else in the page without having to pass javascript objects around. It's just based on using the same string for the for attribute and the id attribute. This allowed us to easily open our login dialog with CSS from anywhere within the UI without having to manage passing around a javascript object/function/method in order to open the dialog. We've PRed #9813 which includes an approach which would make passing around JS modal object easier to do. But in the meantime we've added a little 'hack' here using an additional element and a change listener which allows us to keep this label/input characteristic of our old modals. I'd originally thought this would be a temporary amend in order to wait on #9813 but the more I think about it, the more I think its quite a nice thing to keep - so longer term we may/may not keep this. --- .changelog/9819.txt | 3 + .../consul/intention/form/fieldsets/index.hbs | 9 +- .../intention/form/fieldsets/layout.scss | 2 +- .../consul/intention/form/index.hbs | 6 +- .../components/consul/intention/form/index.js | 4 +- .../app/components/hashicorp-consul/index.hbs | 36 +++--- .../app/components/hashicorp-consul/index.js | 5 + .../app/components/modal-dialog/index.hbs | 78 ++++++------ .../app/components/modal-dialog/index.js | 119 +++--------------- .../app/components/modal-dialog/index.scss | 15 +-- .../app/components/modal-dialog/layout.scss | 72 +++++------ .../app/components/modal-dialog/skin.scss | 18 +-- .../app/components/modal-layer/index.hbs | 6 +- .../app/components/modal-layer/index.js | 20 --- .../app/components/policy-selector/index.hbs | 9 +- .../components/policy-selector/pageobject.js | 2 +- .../app/components/role-selector/index.hbs | 8 +- .../components/role-selector/pageobject.js | 2 +- .../topology-metrics/series/index.hbs | 13 +- ui/packages/consul-ui/package.json | 1 + .../acceptance/dc/intentions/create.feature | 4 +- .../dc/intentions/permissions/create.feature | 2 +- .../dc/intentions/permissions/warn.feature | 8 +- ui/packages/consul-ui/tests/pages.js | 3 +- .../tests/pages/dc/intentions/edit.js | 4 +- .../consul-ui/tests/steps/assertions/page.js | 6 +- ui/yarn.lock | 12 ++ 27 files changed, 198 insertions(+), 269 deletions(-) create mode 100644 .changelog/9819.txt delete mode 100644 ui/packages/consul-ui/app/components/modal-layer/index.js diff --git a/.changelog/9819.txt b/.changelog/9819.txt new file mode 100644 index 0000000000..4b5f6986cc --- /dev/null +++ b/.changelog/9819.txt @@ -0,0 +1,3 @@ +```release-note:improvement +ui: improve accessibility of modal dialogs +``` diff --git a/ui/packages/consul-ui/app/components/consul/intention/form/fieldsets/index.hbs b/ui/packages/consul-ui/app/components/consul/intention/form/fieldsets/index.hbs index ee1894d333..cae38aaeee 100644 --- a/ui/packages/consul-ui/app/components/consul/intention/form/fieldsets/index.hbs +++ b/ui/packages/consul-ui/app/components/consul/intention/form/fieldsets/index.hbs @@ -147,7 +147,7 @@ @@ -156,7 +156,7 @@ {{else}} @@ -190,11 +190,11 @@ -{{#if shouldShowPermissionForm}} +

Edit Permission

@@ -225,6 +225,5 @@
-{{/if}} \ No newline at end of file diff --git a/ui/packages/consul-ui/app/components/consul/intention/form/fieldsets/layout.scss b/ui/packages/consul-ui/app/components/consul/intention/form/fieldsets/layout.scss index fa9353f06c..b098746a7d 100644 --- a/ui/packages/consul-ui/app/components/consul/intention/form/fieldsets/layout.scss +++ b/ui/packages/consul-ui/app/components/consul/intention/form/fieldsets/layout.scss @@ -22,6 +22,6 @@ float: right; } } -.consul-intention-permission-modal [role="dialog"] > div > div { +.consul-intention-permission-modal [role="dialog"] { width: 100%; } diff --git a/ui/packages/consul-ui/app/components/consul/intention/form/index.hbs b/ui/packages/consul-ui/app/components/consul/intention/form/index.hbs index 4860bf5095..193fbf047d 100644 --- a/ui/packages/consul-ui/app/components/consul/intention/form/index.hbs +++ b/ui/packages/consul-ui/app/components/consul/intention/form/index.hbs @@ -34,13 +34,12 @@ as |api|> {{#let api.data as |item|}} {{#if (can 'write intention' item=item)}} -{{#if this.warn}} {{#let (changeset-get item 'Action') as |newAction|}} + as |modal|> +

Set intention to {{newAction}}?

@@ -69,7 +68,6 @@ as |api|>
{{/let}} -{{/if}} {{#let components.AuthForm components.AuthProfile as |AuthForm AuthProfile|}} - - - + +

Log in with a different token

- - + + - - @@ -271,7 +277,9 @@ <:main> - {{yield}} + {{yield (hash + modal=this.modal + )}} <:content-info> diff --git a/ui/packages/consul-ui/app/components/hashicorp-consul/index.js b/ui/packages/consul-ui/app/components/hashicorp-consul/index.js index c65e1efd56..fecd7bcf40 100644 --- a/ui/packages/consul-ui/app/components/hashicorp-consul/index.js +++ b/ui/packages/consul-ui/app/components/hashicorp-consul/index.js @@ -17,4 +17,9 @@ export default class HashiCorpConsul extends Component { this.modal.close(); this.args.onchange(e); } + + @action + keypressClick(e) { + e.target.dispatchEvent(new MouseEvent('click')); + } } diff --git a/ui/packages/consul-ui/app/components/modal-dialog/index.hbs b/ui/packages/consul-ui/app/components/modal-dialog/index.hbs index 8cb052be49..80f1e2fc17 100644 --- a/ui/packages/consul-ui/app/components/modal-dialog/index.hbs +++ b/ui/packages/consul-ui/app/components/modal-dialog/index.hbs @@ -1,51 +1,55 @@ -{{on-window 'resize' (action "resize") }} {{yield}} +