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

- Role of the ui5-select is changed to combobox which change the speech output near to the default behavior. 
- The issue where role="button" can't be used with aria-required / required attribute is fixed. 
- Overwriting the aria-label speech output with aria-labelledby is fixed.

Fixes: #2485
Fixes: #2339
Fixes: #2142

Author: nnaydenow

packages/main/src/Select.hbs

@@ -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

packages/main/src/Select.js

@@ -320,6 +320,7 @@ class Select extends UI5Element {
 
 	_onfocusout() {
 		this.focused = false;
+		this.itemSelectionAnnounce();
 	}
 
 	get _isPickerOpen() {
@@ -488,6 +489,7 @@ class Select extends UI5Element {
 
 	_handleArrowNavigation(event, shouldFireEvent) {
 		let nextIndex = -1;
+		const currentIndex = this._selectedIndex;
 		const isDownKey = isDown(event);
 		const isUpKey = isUp(event);
 
@@ -503,6 +505,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]);
 			}
@@ -637,6 +643,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) {

packages/main/src/themes/Select.css

@@ -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 {

packages/main/test/specs/Select.spec.js

@@ -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";
 
@@ -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";
@@ -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";
 
@@ -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";
@@ -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");
 
@@ -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");
 
@@ -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);
@@ -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);
@@ -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");
 	});
 
@@ -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", () => {
@@ -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";
 
@@ -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";