Skip to content

Commit

Permalink
fix(ui5-combobox, ui5-multi-combobox): accessibility improvements
Browse files Browse the repository at this point in the history
  • Loading branch information
elenastoyanovaa authored and ivoplashkov committed Oct 23, 2024
1 parent db3eae0 commit f0f887b
Show file tree
Hide file tree
Showing 14 changed files with 71 additions and 14 deletions.
2 changes: 1 addition & 1 deletion packages/fiori/src/WizardTab.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
<div class="ui5-wiz-step-main">
<div class="ui5-wiz-step-icon-circle">
{{#if icon}}
<ui5-icon class="ui5-wiz-step-icon" name="{{icon}}"></ui5-icon>
<ui5-icon class="ui5-wiz-step-icon" mode="Decorative" name="{{icon}}"></ui5-icon>
{{else}}
<span class="ui5-wiz-step-number">{{number}}</span>
{{/if}}
Expand Down
1 change: 1 addition & 0 deletions packages/main/src/ComboBox.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
aria-describedby="value-state-description"
aria-label="{{ariaLabelText}}"
aria-required="{{required}}"
aria-controls="{{responsivePopoverId}}"
autocomplete="off"
data-sap-focus-ref
/>
Expand Down
4 changes: 4 additions & 0 deletions packages/main/src/ComboBox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1237,6 +1237,10 @@ class ComboBox extends UI5Element {
return ComboBox.i18nBundle.getText(INPUT_CLEAR_ICON_ACC_NAME);
}

get responsivePopoverId() {
return `${this._id}-popover`;
}

static async onDefine() {
ComboBox.i18nBundle = await getI18nBundle("@ui5/webcomponents");
}
Expand Down
2 changes: 1 addition & 1 deletion packages/main/src/ComboBoxPopover.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
</ui5-input>
</div>
</div>

{{#if hasValueStateText}}
<div class="{{classes.popoverValueState}}" style="{{styles.popoverValueStateMessage}}">
<ui5-icon class="ui5-input-value-state-message-icon" name="{{_valueStateMessageIcon}}"></ui5-icon>
Expand Down
1 change: 1 addition & 0 deletions packages/main/src/MultiComboBox.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
aria-describedby="{{ariaDescribedByText}}"
aria-required="{{required}}"
aria-label="{{ariaLabelText}}"
aria-controls="{{responsivePopoverId}}"
autocomplete="off"
data-sap-focus-ref
/>
Expand Down
4 changes: 4 additions & 0 deletions packages/main/src/MultiComboBox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1921,6 +1921,10 @@ class MultiComboBox extends UI5Element {
return MultiComboBox.i18nBundle.getText(MCB_SELECTED_ITEMS, selected.length, items.length);
}

get responsivePopoverId() {
return `${this._id}-popover`;
}

get classes(): ClassMap {
return {
popover: {
Expand Down
1 change: 1 addition & 0 deletions packages/main/src/MultiComboBoxPopover.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
placement-type="Bottom"
horizontal-align="Left"
class="{{classes.popover}}"
id="{{responsivePopoverId}}"
hide-arrow
_disable-initial-focus
style="{{styles.suggestionsPopover}}"
Expand Down
1 change: 1 addition & 0 deletions packages/main/src/MultiInput.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
@mouseup={{valueHelpMouseUp}}
input-icon
name="value-help"
accessible-name="{{valueHelpLabel}}"
></ui5-icon>
{{/if}}
{{/inline}}
6 changes: 5 additions & 1 deletion packages/main/src/MultiInput.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
} from "@ui5/webcomponents-base/dist/Keys.js";
import type { ITabbable } from "@ui5/webcomponents-base/dist/delegate/ItemNavigation.js";
import { getScopedVarName } from "@ui5/webcomponents-base/dist/CustomElementsScope.js";
import { MULTIINPUT_ROLEDESCRIPTION_TEXT } from "./generated/i18n/i18n-defaults.js";
import { MULTIINPUT_ROLEDESCRIPTION_TEXT, MULTIINPUT_VALUE_HELP_LABEL } from "./generated/i18n/i18n-defaults.js";
import Input from "./Input.js";
import MultiInputTemplate from "./generated/templates/MultiInputTemplate.lit.js";
import styles from "./generated/themes/MultiInput.css.js";
Expand Down Expand Up @@ -412,6 +412,10 @@ class MultiInput extends Input {
};
}

get valueHelpLabel() {
return MultiInput.i18nBundle.getText(MULTIINPUT_VALUE_HELP_LABEL);
}

get ariaRoleDescription() {
return MultiInput.i18nBundle.getText(MULTIINPUT_ROLEDESCRIPTION_TEXT);
}
Expand Down
4 changes: 2 additions & 2 deletions packages/main/src/RangeSlider.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
aria-labelledby="{{_ariaLabelledByStartHandleRefs}}"
aria-disabled="{{_ariaDisabled}}"
>
<ui5-icon name="direction-arrows" slider-icon></ui5-icon>
<ui5-icon name="direction-arrows" mode="Decorative" slider-icon></ui5-icon>

{{#if showTooltip}}
<div class="ui5-slider-tooltip ui5-slider-tooltip--start" style="{{styles.tooltip}}">
Expand All @@ -66,7 +66,7 @@
aria-labelledby="{{_ariaLabelledByEndHandleRefs}}"
aria-disabled="{{_ariaDisabled}}"
>
<ui5-icon name="direction-arrows" slider-icon></ui5-icon>
<ui5-icon name="direction-arrows" mode="Decorative" slider-icon></ui5-icon>

{{#if showTooltip}}
<div class="ui5-slider-tooltip ui5-slider-tooltip--end" style="{{styles.tooltip}}">
Expand Down
2 changes: 1 addition & 1 deletion packages/main/src/Slider.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
data-sap-focus-ref
part="handle"
>
<ui5-icon name="direction-arrows" part="icon-slider" slider-icon></ui5-icon>
<ui5-icon mode="Decorative" "direction-arrows" part="icon-slider" slider-icon></ui5-icon>
{{#if showTooltip}}
<div class="ui5-slider-tooltip" style="{{styles.tooltip}}">
<span class="ui5-slider-tooltip-value">{{tooltipValue}}</span>
Expand Down
3 changes: 3 additions & 0 deletions packages/main/src/i18n/messagebundle.properties
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,9 @@ MULTIINPUT_ROLEDESCRIPTION_TEXT=Multi Value Input
#XFLD: Token number indicator which is used to show more tokens in Tokenizers inside MultiInput and MultiComboBox
MULTIINPUT_SHOW_MORE_TOKENS={0} More

#XACT: ARIA announcement for value help
MULTIINPUT_VALUE_HELP_LABEL=Show Value Help

#XTOL: Tooltip for panel expand title
PANEL_ICON=Expand/Collapse

Expand Down
21 changes: 21 additions & 0 deletions packages/main/test/specs/ComboBox.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -884,6 +884,27 @@ describe("Accessibility", async () => {
assert.strictEqual(ariaHiddenText.includes("Value State"), true, "Hidden screen reader text is correct");
assert.strictEqual(valueStateText.includes("Custom error"), true, "Displayed value state message text is correct");
});

it("Should render aria-haspopup attribute with value 'dialog'", async () => {
await browser.url(`test/pages/ComboBox.html`);

const combo = await browser.$("#combo");
const innerInput = await combo.shadow$("input");

assert.strictEqual(await innerInput.getAttribute("aria-haspopup"), "dialog", "Should render aria-haspopup attribute with value 'dialog'");
});

it("Should apply aria-controls pointing to the responsive popover", async () => {
await browser.url(`test/pages/ComboBox.html`);

const combo = await browser.$("#combo");
const innerInput = await combo.shadow$("input");
const popover = await combo.shadow$("ui5-responsive-popover");

await combo.scrollIntoView();

assert.strictEqual(await innerInput.getAttribute("aria-controls"), await popover.getAttribute("id"), "aria-controls attribute is correct.");
});
});

describe("Keyboard navigation", async () => {
Expand Down
33 changes: 25 additions & 8 deletions packages/main/test/specs/MultiComboBox.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -695,7 +695,7 @@ describe("MultiComboBox general interaction", () => {

const tokenizerNMore = await cb.shadow$("[ui5-tokenizer]");
const nMoreLabel = await tokenizerNMore.shadow$(".ui5-tokenizer-more-text");

await nMoreLabel.click();

assert.ok(await popover.$(".ui5-mcb-select-all-checkbox").getProperty("checked"), "Select All CheckBox should be selected");
Expand Down Expand Up @@ -1757,32 +1757,32 @@ describe("MultiComboBox general interaction", () => {

it ("Should check clear icon availability", async () => {
await browser.url(`test/pages/MultiComboBox.html`);

const cb = await $("#clear-icon-cb");
const inner = cb.shadow$("input");
const clearIcon = await cb.shadow$(".ui5-input-clear-icon-wrapper");

assert.notOk(await cb.getProperty("_effectiveShowClearIcon"), "_effectiveShowClearIcon should be set to false when mcb has no value");

await inner.click();
await inner.keys("c");

assert.ok(await cb.getProperty("_effectiveShowClearIcon"), "_effectiveShowClearIcon should be set to true upon typing");
});

it ("Should check clear icon events", async () => {
await browser.url(`test/pages/MultiComboBox.html`);

const cb = await $("#clear-icon-cb");

await cb.shadow$("input").click();
await cb.shadow$("input").keys("c");

const clearIcon = await cb.shadow$(".ui5-input-clear-icon-wrapper");

// focus out the combo
await clearIcon.click();

assert.strictEqual(await $("#clear-icon-change-count").getText(), "0", "change event is not fired");
assert.strictEqual(await $("#clear-icon-input-count").getText(), "2", "input event is fired twice");
});
Expand Down Expand Up @@ -1928,6 +1928,23 @@ describe("MultiComboBox general interaction", () => {
assert.strictEqual(await innerInput.getAttribute("aria-label"), await mcbLabel.getHTML(false), "aria-label attribute is correct.");
});

it("Should apply aria-controls pointing to the responsive popover", async () => {
const mcb = await browser.$("#mcb-predefined-value");
const innerInput = await mcb.shadow$("input");
const popover = await mcb.shadow$("ui5-responsive-popover");

await mcb.scrollIntoView();

assert.strictEqual(await innerInput.getAttribute("aria-controls"), await popover.getAttribute("id"), "aria-controls attribute is correct.");
});

it("Should render aria-haspopup attribute with value 'dialog'", async () => {
const mcb = await browser.$("#mcb-compact");
const innerInput = await mcb.shadow$("input");

assert.strictEqual(await innerInput.getAttribute("aria-haspopup"), "dialog", "Should render aria-haspopup attribute with value 'dialog'");
});

it("Value state type should be added to the screen readers default value states announcement", async () => {
await browser.url(`test/pages/MultiComboBox.html`);

Expand Down

0 comments on commit f0f887b

Please sign in to comment.