From 1dca3ad6b4e9737806c064bf7435e5a46db8ad13 Mon Sep 17 00:00:00 2001 From: Petar Dimov Date: Thu, 11 Mar 2021 16:57:36 +0200 Subject: [PATCH 1/7] wip(ui5-busyindicator): ACC improvements - Tooltip is now shown on hover - Focus is now on the correct div, so screen readers can read it properly Fixes #2381 --- packages/main/src/BusyIndicator.hbs | 16 ++++++++-- packages/main/src/BusyIndicator.js | 37 ++++++++++++++++------ packages/main/src/themes/BusyIndicator.css | 7 ++++ 3 files changed, 48 insertions(+), 12 deletions(-) diff --git a/packages/main/src/BusyIndicator.hbs b/packages/main/src/BusyIndicator.hbs index b5e335e62bee..eb512e4bd5ba 100644 --- a/packages/main/src/BusyIndicator.hbs +++ b/packages/main/src/BusyIndicator.hbs @@ -1,7 +1,15 @@
{{#if active}} -
+
+
@@ -15,4 +23,8 @@
-
+ + {{#if active}} +
+ {{/if}} +
\ No newline at end of file diff --git a/packages/main/src/BusyIndicator.js b/packages/main/src/BusyIndicator.js index 82e678d82c9b..3cafd31e64f9 100644 --- a/packages/main/src/BusyIndicator.js +++ b/packages/main/src/BusyIndicator.js @@ -1,6 +1,7 @@ import UI5Element from "@ui5/webcomponents-base/dist/UI5Element.js"; import litRender from "@ui5/webcomponents-base/dist/renderer/LitRenderer.js"; import { isIE } from "@ui5/webcomponents-base/dist/Device.js"; +import { isTabNext, isTabPrevious } from "@ui5/webcomponents-base/dist/Keys.js"; import { fetchI18nBundle, getI18nBundle } from "@ui5/webcomponents-base/dist/i18nBundle.js"; import BusyIndicatorSize from "./types/BusyIndicatorSize.js"; import Label from "./Label.js"; @@ -110,14 +111,6 @@ class BusyIndicator extends UI5Element { this._preventHandler = this._preventEvent.bind(this); } - onBeforeRendering() { - if (this.active) { - this.tabIndex = -1; - } else { - this.removeAttribute("tabindex"); - } - } - onEnterDOM() { this.addEventListener("keyup", this._preventHandler, { capture: true, @@ -171,8 +164,32 @@ class BusyIndicator extends UI5Element { } _preventEvent(event) { - if (this.active) { - event.stopImmediatePropagation(); + if (!this.active) { + return; + } + + event.stopImmediatePropagation(); + + // Special handling for "tab" keydown: redirect to outer element before or after this + if (event.type === "keydown" && (isTabNext(event) || isTabPrevious(event))) { + const tabbable = isTabPrevious(event) ? this.shadowRoot.querySelector(".first-tabbable") : this.shadowRoot.querySelector(".last-tabbable") + + tabbable.setAttribute("tabindex", -1); + this._focusingOuterElement = true; + console.warn("focus outer el", tabbable) + tabbable.focus(); + this._focusingOuterElement = false; + tabbable.setAttribute("tabindex", 0); + } + } + + /** + * Moves focus to the busy indicator + */ + _ontabbablefocusin() { + if (!this._focusingOuterElement) { + this.shadowRoot.querySelector(".ui5-busyindicator-dynamic-content").focus(); + console.error("move focus to busy area") } } } diff --git a/packages/main/src/themes/BusyIndicator.css b/packages/main/src/themes/BusyIndicator.css index 88632cdecdb4..3461e5b9cdc8 100644 --- a/packages/main/src/themes/BusyIndicator.css +++ b/packages/main/src/themes/BusyIndicator.css @@ -8,6 +8,9 @@ :host([active]) { color: var(--sapContent_IconColor); +} + +:host([active]) ::slotted(*) { pointer-events: none; } @@ -107,6 +110,10 @@ background-color: inherit; } +.ui5-busyindicator-dynamic-content:focus { + outline: 1px dotted var(--sapContent_FocusColor); +} + .circle-animation-0 { animation: grow 1.6s infinite cubic-bezier(0.32, 0.06, 0.85, 1.11); } From e7f722cf884ad32ff499dc438d4db50a916e703b Mon Sep 17 00:00:00 2001 From: Petar Dimov Date: Thu, 11 Mar 2021 17:25:20 +0200 Subject: [PATCH 2/7] set tabindex to the slot --- packages/main/src/BusyIndicator.hbs | 8 ++------ packages/main/src/BusyIndicator.js | 32 ++++++----------------------- 2 files changed, 8 insertions(+), 32 deletions(-) diff --git a/packages/main/src/BusyIndicator.hbs b/packages/main/src/BusyIndicator.hbs index eb512e4bd5ba..6683c290841a 100644 --- a/packages/main/src/BusyIndicator.hbs +++ b/packages/main/src/BusyIndicator.hbs @@ -1,7 +1,6 @@
{{#if active}} -
@@ -22,9 +22,5 @@ {{/if}}
- - - {{#if active}} -
- {{/if}} +
\ No newline at end of file diff --git a/packages/main/src/BusyIndicator.js b/packages/main/src/BusyIndicator.js index 3cafd31e64f9..468869f0004b 100644 --- a/packages/main/src/BusyIndicator.js +++ b/packages/main/src/BusyIndicator.js @@ -1,7 +1,6 @@ import UI5Element from "@ui5/webcomponents-base/dist/UI5Element.js"; import litRender from "@ui5/webcomponents-base/dist/renderer/LitRenderer.js"; import { isIE } from "@ui5/webcomponents-base/dist/Device.js"; -import { isTabNext, isTabPrevious } from "@ui5/webcomponents-base/dist/Keys.js"; import { fetchI18nBundle, getI18nBundle } from "@ui5/webcomponents-base/dist/i18nBundle.js"; import BusyIndicatorSize from "./types/BusyIndicatorSize.js"; import Label from "./Label.js"; @@ -163,35 +162,16 @@ class BusyIndicator extends UI5Element { }; } - _preventEvent(event) { - if (!this.active) { - return; - } - - event.stopImmediatePropagation(); - - // Special handling for "tab" keydown: redirect to outer element before or after this - if (event.type === "keydown" && (isTabNext(event) || isTabPrevious(event))) { - const tabbable = isTabPrevious(event) ? this.shadowRoot.querySelector(".first-tabbable") : this.shadowRoot.querySelector(".last-tabbable") - - tabbable.setAttribute("tabindex", -1); - this._focusingOuterElement = true; - console.warn("focus outer el", tabbable) - tabbable.focus(); - this._focusingOuterElement = false; - tabbable.setAttribute("tabindex", 0); - } + get slotTabIndex() { + return this.active ? -1 : 0; } - /** - * Moves focus to the busy indicator - */ - _ontabbablefocusin() { - if (!this._focusingOuterElement) { - this.shadowRoot.querySelector(".ui5-busyindicator-dynamic-content").focus(); - console.error("move focus to busy area") + _preventEvent(event) { + if (this.active) { + event.stopImmediatePropagation(); } } + } BusyIndicator.define(); From 04f36555e218f9534dcef7127991675b03ecfb4b Mon Sep 17 00:00:00 2001 From: Petar Dimov Date: Fri, 12 Mar 2021 10:20:43 +0200 Subject: [PATCH 3/7] include text property in the announcement --- packages/main/src/BusyIndicator.hbs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/main/src/BusyIndicator.hbs b/packages/main/src/BusyIndicator.hbs index 6683c290841a..b76f75731375 100644 --- a/packages/main/src/BusyIndicator.hbs +++ b/packages/main/src/BusyIndicator.hbs @@ -9,6 +9,7 @@ aria-valuemin="0" aria-valuemax="100" aria-valuetext="Busy" + aria-labelledby="{{_id}}-label" >
@@ -16,7 +17,7 @@
{{/if}} {{#if text}} - + {{text}} {{/if}} From 11c8e48f2e7d21634a9f1bd18181f0cb8c5c5a36 Mon Sep 17 00:00:00 2001 From: Petar Dimov Date: Mon, 15 Mar 2021 18:03:24 +0200 Subject: [PATCH 4/7] add testss --- packages/main/test/pages/BusyIndicator.html | 4 +-- .../main/test/specs/BusyIndicator.spec.js | 31 +++++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/packages/main/test/pages/BusyIndicator.html b/packages/main/test/pages/BusyIndicator.html index 6ccd82c951c8..2d110e41dadb 100644 --- a/packages/main/test/pages/BusyIndicator.html +++ b/packages/main/test/pages/BusyIndicator.html @@ -40,12 +40,12 @@

- +

- + Hello World diff --git a/packages/main/test/specs/BusyIndicator.spec.js b/packages/main/test/specs/BusyIndicator.spec.js index 0cfa3c7ea08f..ea13f830a8d4 100644 --- a/packages/main/test/specs/BusyIndicator.spec.js +++ b/packages/main/test/specs/BusyIndicator.spec.js @@ -15,4 +15,35 @@ describe("BusyIndicator general interaction", () => { assert.strictEqual(input.getProperty("value"), "0", "itemClick is not thrown"); }); + it("tests focus handling", () => { + const busyIndicator = browser.$("#indicator1"); + busyIndicator.click(); + + let innerFocusElement = browser.execute(() => { + return document.getElementById("indicator1").shadowRoot.activeElement; + }); + + innerFocusElement = $(innerFocusElement); + + assert.strictEqual(innerFocusElement.getAttribute("class"), "ui5-busyindicator-dynamic-content", "The correct inner element is focused"); + }); + + it("tests internal focused element attributes", () => { + const busyIndicator = browser.$("#indicator1"); + busyIndicator.click(); + const innerFocusElement = busyIndicator.shadow$(".ui5-busyindicator-dynamic-content"); + + assert.strictEqual(innerFocusElement.getAttribute("role"), "progressbar", "Correct 'role' is set"); + assert.strictEqual(innerFocusElement.getAttribute("tabindex"), "0", "Correct 'tabindex' is set"); + assert.strictEqual(innerFocusElement.getAttribute("aria-valuemin"), "0", "Correct 'aria-valuemin' is set"); + assert.strictEqual(innerFocusElement.getAttribute("aria-valuemax"), "100", "Correct 'aria-valuemax' is set"); + assert.strictEqual(innerFocusElement.getAttribute("aria-valuetext"), "Busy", "Correct 'aria-valuetext' is set"); + }); + + it("tests content is not reachable with keyboard when active", () => { + const busyIndicator = browser.$("#indicatorWithBtn"); + const defaultSLot = busyIndicator.shadow$("slot"); + + assert.strictEqual(defaultSLot.getAttribute("tabindex"), "-1", "Slot is not reachable via keyboard"); + }); }); From f9dc97bcef0d856bd8d5374bb7b759b3fa68d774 Mon Sep 17 00:00:00 2001 From: Petar Dimov Date: Thu, 18 Mar 2021 12:09:12 +0200 Subject: [PATCH 5/7] eslint --- packages/main/src/BusyIndicator.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/main/src/BusyIndicator.js b/packages/main/src/BusyIndicator.js index 468869f0004b..9246ecbb4e5f 100644 --- a/packages/main/src/BusyIndicator.js +++ b/packages/main/src/BusyIndicator.js @@ -171,7 +171,6 @@ class BusyIndicator extends UI5Element { event.stopImmediatePropagation(); } } - } BusyIndicator.define(); From 8696832ed3562abb67707b771addcd561915169c Mon Sep 17 00:00:00 2001 From: Petar Dimov Date: Thu, 18 Mar 2021 13:32:57 +0200 Subject: [PATCH 6/7] stretch busy area to the full width/height --- packages/main/src/themes/BusyIndicator.css | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/packages/main/src/themes/BusyIndicator.css b/packages/main/src/themes/BusyIndicator.css index 3461e5b9cdc8..134274a5f59c 100644 --- a/packages/main/src/themes/BusyIndicator.css +++ b/packages/main/src/themes/BusyIndicator.css @@ -10,10 +10,6 @@ color: var(--sapContent_IconColor); } -:host([active]) ::slotted(*) { - pointer-events: none; -} - :host([active]) :not(.ui5-busyindicator-root--ie) ::slotted(:not([class^="ui5-busyindicator-"])) { opacity: 0.6; } @@ -81,12 +77,10 @@ .ui5-busyindicator-wrapper { position: absolute; z-index: 99; - width: 100%; - /* Fixes IE positioning */ left: 0; right: 0; - top: 50%; - transform: translate(0, -50%); + top: 0; + bottom: 0; } .ui5-busyindicator-circle { @@ -112,6 +106,7 @@ .ui5-busyindicator-dynamic-content:focus { outline: 1px dotted var(--sapContent_FocusColor); + outline-offset: -2px; } .circle-animation-0 { From 8b5513c035b6f8abce3ab7015199d1a97c5048f0 Mon Sep 17 00:00:00 2001 From: Petar Dimov Date: Wed, 31 Mar 2021 14:44:37 +0300 Subject: [PATCH 7/7] fix positioning of custom text --- packages/main/src/BusyIndicator.hbs | 38 +++++++++---------- packages/main/src/themes/BusyIndicator.css | 33 ++++++++-------- packages/main/test/pages/BusyIndicator.html | 10 ++++- .../main/test/specs/BusyIndicator.spec.js | 4 +- 4 files changed, 45 insertions(+), 40 deletions(-) diff --git a/packages/main/src/BusyIndicator.hbs b/packages/main/src/BusyIndicator.hbs index b76f75731375..3c71b3c23ca1 100644 --- a/packages/main/src/BusyIndicator.hbs +++ b/packages/main/src/BusyIndicator.hbs @@ -1,27 +1,27 @@
-
- {{#if active}} -
+ {{#if active}} +
+
- {{/if}} - {{#if text}} - - {{text}} - - {{/if}} -
+ {{#if text}} + + {{text}} + + {{/if}} +
+ {{/if}}
\ No newline at end of file diff --git a/packages/main/src/themes/BusyIndicator.css b/packages/main/src/themes/BusyIndicator.css index 134274a5f59c..ddc197dcccac 100644 --- a/packages/main/src/themes/BusyIndicator.css +++ b/packages/main/src/themes/BusyIndicator.css @@ -2,10 +2,6 @@ display: inline-block; } -:host(:not([active])) .ui5-busyindicator-wrapper { - display: none; -} - :host([active]) { color: var(--sapContent_IconColor); } @@ -74,13 +70,27 @@ background-color: inherit; } -.ui5-busyindicator-wrapper { +.ui5-busyindicator-busy-area { position: absolute; z-index: 99; left: 0; right: 0; top: 0; bottom: 0; + display: flex; + justify-content: center; + align-items: center; + background-color: inherit; + flex-direction: column; +} + +.ui5-busyindicator-busy-area:focus { + outline: 1px dotted var(--sapContent_FocusColor); + outline-offset: -2px; +} + +.ui5-busyindicator-circles-wrapper { + line-height: 0; } .ui5-busyindicator-circle { @@ -96,19 +106,6 @@ border-radius: 100%; } -.ui5-busyindicator-dynamic-content { - height: 100%; - display: flex; - justify-content: center; - align-items: center; - background-color: inherit; -} - -.ui5-busyindicator-dynamic-content:focus { - outline: 1px dotted var(--sapContent_FocusColor); - outline-offset: -2px; -} - .circle-animation-0 { animation: grow 1.6s infinite cubic-bezier(0.32, 0.06, 0.85, 1.11); } diff --git a/packages/main/test/pages/BusyIndicator.html b/packages/main/test/pages/BusyIndicator.html index 2d110e41dadb..beaf9838f36a 100644 --- a/packages/main/test/pages/BusyIndicator.html +++ b/packages/main/test/pages/BusyIndicator.html @@ -3,7 +3,7 @@ - + Busy Indicator @@ -79,6 +79,14 @@ + + + Item 1 + Item 2 + Item 3 + + +


diff --git a/packages/main/test/specs/BusyIndicator.spec.js b/packages/main/test/specs/BusyIndicator.spec.js index aefe9733e116..1da46702ee2f 100644 --- a/packages/main/test/specs/BusyIndicator.spec.js +++ b/packages/main/test/specs/BusyIndicator.spec.js @@ -27,13 +27,13 @@ describe("BusyIndicator general interaction", () => { innerFocusElement = $(innerFocusElement); - assert.strictEqual(innerFocusElement.getAttribute("class"), "ui5-busyindicator-dynamic-content", "The correct inner element is focused"); + assert.strictEqual(innerFocusElement.getAttribute("class"), "ui5-busyindicator-busy-area", "The correct inner element is focused"); }); it("tests internal focused element attributes", () => { const busyIndicator = browser.$("#indicator1"); busyIndicator.click(); - const innerFocusElement = busyIndicator.shadow$(".ui5-busyindicator-dynamic-content"); + const innerFocusElement = busyIndicator.shadow$(".ui5-busyindicator-busy-area"); assert.strictEqual(innerFocusElement.getAttribute("role"), "progressbar", "Correct 'role' is set"); assert.strictEqual(innerFocusElement.getAttribute("tabindex"), "0", "Correct 'tabindex' is set");