Skip to content

fix(ui5-select): correct role and screen reader speech out #2587

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Jan 4, 2021
Merged

fix(ui5-select): correct role and screen reader speech out #2587

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2e779b4
fix(ui5-select): correct role and screen reader speech out
nnaydenow Dec 15, 2020
fddebb6
fix(ui5-select): correct role and screen reader speech out
nnaydenow Dec 16, 2020
7dead1f
align to the new aria-standanrd (POC)
nnaydenow Dec 18, 2020
338d307
align to new aria standard (POC)
nnaydenow Dec 21, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 17 additions & 15 deletions packages/main/src/Select.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,26 +1,28 @@
<div
class="ui5-select-root"
dir="{{effectiveDir}}"
tabindex="{{tabIndex}}"
id="{{_id}}-select"
role="button"
aria-haspopup="listbox"
aria-label="{{ariaLabelText}}"
aria-labelledby="{{_id}}-label"
aria-describedby="{{valueStateTextId}}"
aria-disabled="{{isDisabled}}"
aria-required="{{required}}"
aria-expanded="{{_isPickerOpen}}"
@keydown="{{_onkeydown}}"
@keyup="{{_onkeyup}}"
@focusin="{{_onfocusin}}"
@focusout="{{_onfocusout}}"
@click="{{_toggleRespPopover}}"
>
<div class="ui5-select-label-root">
<ui5-label id="{{_id}}-label" for="{{_id}}-select">{{_text}}</ui5-label>
<div class="ui5-select-label-root"
tabindex="{{tabIndex}}"
role="combobox"
aria-haspopup="listbox"
aria-label="{{ariaLabelText}}"
aria-describedby="{{valueStateTextId}}"
aria-disabled="{{isDisabled}}"
aria-required="{{required}}"
aria-expanded="{{_isPickerOpen}}"
@keydown="{{_onkeydown}}"
@keyup="{{_onkeyup}}"
@focusin="{{_onfocusin}}"
@focusout="{{_onfocusout}}"
>
{{_text}}
</div>

<span id="{{_id}}-selectionText" class="ui5-hidden-text" aria-live="polite" role="status"></span>

<ui5-icon
name="slim-arrow-down"
input-icon
Expand Down
16 changes: 16 additions & 0 deletions packages/main/src/Select.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,7 @@ class Select extends UI5Element {

_onfocusout() {
this.focused = false;
this.itemSelectionAnnounce();
}

get _isPickerOpen() {
Expand Down Expand Up @@ -478,6 +479,7 @@ class Select extends UI5Element {

_handleArrowNavigation(event, shouldFireEvent) {
let nextIndex = -1;
const currentIndex = this._selectedIndex;
const isDownKey = isDown(event);
const isUpKey = isUp(event);

Expand All @@ -493,6 +495,10 @@ class Select extends UI5Element {
this.options[nextIndex].selected = true;
this._selectedIndex = nextIndex === -1 ? this._selectedIndex : nextIndex;

if (currentIndex !== this._selectedIndex) {
this.itemSelectionAnnounce();
}

if (shouldFireEvent) {
this._fireChangeEvent(this.options[nextIndex]);
}
Expand Down Expand Up @@ -627,6 +633,16 @@ class Select extends UI5Element {
return isPhone();
}

itemSelectionAnnounce() {
const invisibleText = this.shadowRoot.querySelector(`#${this._id}-selectionText`);

if (this.focused && !this._isPickerOpen && this._currentlySelectedOption) {
invisibleText.textContent = this._currentlySelectedOption.textContent;
} else {
invisibleText.textContent = "";
}
}

async openValueStatePopover() {
this.popover = await this._getPopover();
if (this.popover) {
Expand Down
15 changes: 9 additions & 6 deletions packages/main/src/themes/Select.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,20 @@
}

.ui5-select-label-root {
display: inline-flex;
align-items: center;
flex-shrink: 1;
flex-grow: 1;
height: 100%;
align-self: center;
min-width: 1rem;
padding-left: 0.5rem;
}

.ui5-select-label-root [ui5-label] {
cursor: pointer;
outline: none;
white-space: nowrap;
overflow: hidden;
text-overflow: ellipsis;
color: var(--sapContent_LabelColor);
font-family: "72override", var(--sapFontFamily);
font-size: var(--sapFontSize);
font-weight: normal;
}

:host [dir="rtl"].ui5-select-root .ui5-select-label-root {
Expand Down
64 changes: 48 additions & 16 deletions packages/main/test/specs/Select.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ describe("Select general interaction", () => {

it("fires change on selection", () => {
const select = $("#mySelect");
const selectText = browser.$("#mySelect").shadow$("ui5-label");
const selectText = browser.$("#mySelect").shadow$(".ui5-select-label-root");
const inputResult = browser.$("#inputResult").shadow$("input");
const EXPECTED_SELECTION_TEXT = "Cozy";

Expand Down Expand Up @@ -34,7 +34,7 @@ describe("Select general interaction", () => {

it("fires change on selection with keyboard handling", () => {
const select = $("#mySelect2").shadow$(".ui5-select-root");
const selectText = browser.$("#mySelect2").shadow$("ui5-label");
const selectText = browser.$("#mySelect2").shadow$(".ui5-select-label-root");
const inputResult = browser.$("#inputResult");
const EXPECTED_SELECTION_TEXT1 = "Compact";
const EXPECTED_SELECTION_TEXT2 = "Condensed";
Expand All @@ -57,7 +57,7 @@ describe("Select general interaction", () => {
it("changes selection while closed with Arrow Up/Down", () => {
const inputResult = browser.$("#inputResult").shadow$("input");
const select = $("#mySelect2");
const selectText = browser.$("#mySelect2").shadow$("ui5-label");
const selectText = browser.$("#mySelect2").shadow$(".ui5-select-label-root");
const EXPECTED_SELECTION_TEXT1 = "Compact";
const EXPECTED_SELECTION_TEXT2 = "Condensed";

Expand All @@ -74,6 +74,39 @@ describe("Select general interaction", () => {
assert.strictEqual(inputResult.getProperty("value"), "5", "Change event should have fired twice");
});

it("changes selection sync with selection announcement", () => {
const btn = $("#myBtn2");
const inputResult = browser.$("#inputResult").shadow$("input");
const select = $("#mySelect2");
const selectId = select.getProperty("_id")
const selectText = browser.$("#mySelect2").shadow$(".ui5-select-label-root");
const selectionText = browser.$("#mySelect2").shadow$(`#${selectId}-selectionText`);
const EXPECTED_SELECTION_TEXT1 = "Compact";
const EXPECTED_SELECTION_TEXT2 = "Condensed";

select.click();
select.keys("Escape");

assert.strictEqual(selectionText.getHTML(false), "", "Selection announcement text should be clear if there is no interaction");

select.keys("ArrowUp");
assert.ok(selectText.getHTML(false).indexOf(EXPECTED_SELECTION_TEXT1), "Arrow Up should change selected item");
assert.strictEqual(selectionText.getHTML(false), EXPECTED_SELECTION_TEXT1, "Selection announcement text should be equalt to the current selected item's text");

select.click();
select.keys("Escape");
assert.strictEqual(selectionText.getHTML(false), "", "Selection announcement text should be cleared if the picker is opened");

select.keys("ArrowDown");
assert.ok(selectText.getHTML(false).indexOf(EXPECTED_SELECTION_TEXT2), "Arrow Up should change selected item");
assert.strictEqual(selectionText.getHTML(false), EXPECTED_SELECTION_TEXT2, "Selection announcement text should be equalt to the current selected item's text");

btn.click();
assert.strictEqual(selectionText.getHTML(false), "", "Selection announcement text should be cleared on focusout");

assert.strictEqual(inputResult.getProperty("value"), "7", "Change event should have fired twice");
});

it("changes selection on Tab", () => {
const select = browser.$("#keyboardHandling");
const EXPECTED_SELECTION_TEXT = "Banana";
Expand All @@ -84,7 +117,7 @@ describe("Select general interaction", () => {

select.keys("ArrowUp");
select.keys("Tab");
const selectText = select.shadow$("ui5-label");
const selectText = select.shadow$(".ui5-select-label-root");

assert.ok(selectText.getHTML(false).indexOf(EXPECTED_SELECTION_TEXT) > -1, "Arrow Up should change selected item");

Expand All @@ -105,7 +138,7 @@ describe("Select general interaction", () => {

select.keys("ArrowDown");
browser.keys(["Shift", "Tab"]);
const selectText = select.shadow$("ui5-label");
const selectText = select.shadow$(".ui5-select-label-root");

assert.ok(selectText.getHTML(false).indexOf(EXPECTED_SELECTION_TEXT) > -1, "Arrow Down should change selected item");

Expand All @@ -119,7 +152,7 @@ describe("Select general interaction", () => {
it("tests selection does not cycle with ArrowDown", () => {
const select = $("#selectionNotCycling");
const EXPECTED_SELECTION_TEXT = "Opt3";
const selectOptionText = select.shadow$("ui5-label");
const selectOptionText = select.shadow$(".ui5-select-label-root");

select.click();
assert.ok(selectOptionText.getHTML(false).indexOf(EXPECTED_SELECTION_TEXT) > -1, "Selected option text is " + EXPECTED_SELECTION_TEXT);
Expand All @@ -135,7 +168,7 @@ describe("Select general interaction", () => {
it("tests selection does not cycle with ArrowUp", () => {
const select = $("#selectionNotCycling2");
const EXPECTED_SELECTION_TEXT = "Opt1";
const selectOptionText = select.shadow$("ui5-label");
const selectOptionText = select.shadow$(".ui5-select-label-root");

select.click();
assert.ok(selectOptionText.getHTML(false).indexOf(EXPECTED_SELECTION_TEXT) > -1, "Selected option text is " + EXPECTED_SELECTION_TEXT);
Expand Down Expand Up @@ -223,18 +256,18 @@ describe("Select general interaction", () => {

it("reverts value before open after clicking on escape", () => {
const select = $("#mySelect");
const selectText = browser.$("#mySelect").shadow$("ui5-label").getHTML(false);
const selectText = browser.$("#mySelect").shadow$(".ui5-select-label-root").getHTML(false);
const inputResult = browser.$("#inputResult").shadow$("input");

select.click();
select.keys("ArrowDown");
select.keys("Escape");

const selectedOption = browser.$("#mySelect ui5-option[selected]");
const selectTextAfterEscape = browser.$("#mySelect").shadow$("ui5-label").getHTML(false);
const selectTextAfterEscape = browser.$("#mySelect").shadow$(".ui5-select-label-root").getHTML(false);

assert.ok(selectedOption.getProperty("selected"), "Initially selected item should remain selected");
assert.strictEqual(inputResult.getProperty("value"), "5", "Change event should not be fired");
assert.strictEqual(inputResult.getProperty("value"), "7", "Change event should not be fired");
assert.strictEqual(selectTextAfterEscape, selectText, "Initially selected item should remain selected");
});

Expand All @@ -250,7 +283,7 @@ describe("Select general interaction", () => {
// focus out select
btn.click();

assert.strictEqual(inputResult.getProperty("value"), "6", "Change event should be fired");
assert.strictEqual(inputResult.getProperty("value"), "8", "Change event should be fired");
});

it("fires change event after selecting a previewed item", () => {
Expand All @@ -268,12 +301,12 @@ describe("Select general interaction", () => {

firstItem.click();

assert.strictEqual(inputResult.getProperty("value"), "7", "Change event should be fired");
assert.strictEqual(inputResult.getProperty("value"), "9", "Change event should be fired");
});

it("tests ESC on closed picker", () => {
const select = $("#mySelectEsc");
const selectText = browser.$("#mySelectEsc").shadow$("ui5-label");
const selectText = browser.$("#mySelectEsc").shadow$(".ui5-select-label-root");
const EXPECTED_SELECTION_TEXT = "Cozy";
const EXPECTED_SELECTION_TEXT2 = "Condensed";

Expand All @@ -294,10 +327,9 @@ describe("Select general interaction", () => {
assert.ok(selectText.getHTML(false).indexOf(EXPECTED_SELECTION_TEXT2) !== -1, "Select label is correct.");
});


it("Tests aria-label and aria-labelledby", () => {
const select1 = browser.$("#textAreaAriaLabel").shadow$(".ui5-select-root");
const select2 = browser.$("#textAreaAriaLabelledBy").shadow$(".ui5-select-root");
const select1 = browser.$("#textAreaAriaLabel").shadow$(".ui5-select-label-root");
const select2 = browser.$("#textAreaAriaLabelledBy").shadow$(".ui5-select-label-root");
const EXPECTED_ARIA_LABEL1 = "Hello World";
const EXPECTED_ARIA_LABEL2 = "info text";

Expand Down