Skip to content

Commit

Permalink
fix(ui5-button): apply aria-expanded to inner button tag (#1781)
Browse files Browse the repository at this point in the history
  • Loading branch information
fifoosid committed Jun 12, 2020
1 parent a04f483 commit df9e4e9
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 40 deletions.
6 changes: 5 additions & 1 deletion packages/base/src/util/isValidPropertyName.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
// Note: disabled is present in IE so we explicitly allow it here.
// Others, such as ariaLabel, we explicitly override, so valid too
const whitelist = ["disabled", "ariaLabel"];
const whitelist = [
"disabled",
"ariaLabel",
"ariaExpanded",
];

/**
* Checks whether a property name is valid (does not collide with existing DOM API properties)
Expand Down
72 changes: 36 additions & 36 deletions packages/main/src/Button.hbs
Original file line number Diff line number Diff line change
@@ -1,42 +1,42 @@
<button
type="button"
class="ui5-button-root"
?disabled="{{disabled}}"
data-sap-focus-ref
{{> ariaPressed}}
dir="{{effectiveDir}}"
@focusout={{_onfocusout}}
@focusin={{_onfocusin}}
@click={{_onclick}}
@mousedown={{_onmousedown}}
@mouseup={{_onmouseup}}
@keydown={{_onkeydown}}
@keyup={{_onkeyup}}
tabindex={{tabIndexValue}}
aria-expanded="{{accInfo.ariaExpanded}}"
aria-controls="{{accInfo.ariaControls}}"
aria-haspopup="{{accInfo.ariaHaspopup}}"
aria-label="{{ariaLabelText}}"
title="{{accInfo.title}}"
part="button"
>
{{#if icon}}
<ui5-icon
class="ui5-button-icon"
name="{{icon}}"
show-tooltip={{iconOnly}}
></ui5-icon>
{{/if}}
type="button"
class="ui5-button-root"
?disabled="{{disabled}}"
data-sap-focus-ref
{{> ariaPressed}}
dir="{{effectiveDir}}"
@focusout={{_onfocusout}}
@focusin={{_onfocusin}}
@click={{_onclick}}
@mousedown={{_onmousedown}}
@mouseup={{_onmouseup}}
@keydown={{_onkeydown}}
@keyup={{_onkeyup}}
tabindex={{tabIndexValue}}
aria-expanded="{{accInfo.ariaExpanded}}"
aria-controls="{{accInfo.ariaControls}}"
aria-haspopup="{{accInfo.ariaHaspopup}}"
aria-label="{{ariaLabelText}}"
title="{{accInfo.title}}"
part="button"
>
{{#if icon}}
<ui5-icon
class="ui5-button-icon"
name="{{icon}}"
show-tooltip={{iconOnly}}
></ui5-icon>
{{/if}}

<span id="{{_id}}-content" class="ui5-button-text">
<bdi>
<slot></slot>
</bdi>
</span>
<span id="{{_id}}-content" class="ui5-button-text">
<bdi>
<slot></slot>
</bdi>
</span>

{{#if hasButtonType}}
<span class="ui5-hidden-text">{{buttonTypeText}}</span>
{{/if}}
{{#if hasButtonType}}
<span class="ui5-hidden-text">{{buttonTypeText}}</span>
{{/if}}
</button>

{{#*inline "ariaPressed"}}{{/inline}}
12 changes: 11 additions & 1 deletion packages/main/src/Button.js
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,16 @@ const metadata = {
defaultValue: "",
},

/**
* @type {String}
* @defaultvalue ""
* @public
* @since 1.0.0-rc.8
*/
ariaExpanded: {
type: String,
},

/**
* Indicates if the element if focusable
* @private
Expand Down Expand Up @@ -340,7 +350,7 @@ class Button extends UI5Element {

get accInfo() {
return {
"ariaExpanded": this._buttonAccInfo && this._buttonAccInfo.ariaExpanded,
"ariaExpanded": this.ariaExpanded || (this._buttonAccInfo && this._buttonAccInfo.ariaExpanded),
"ariaControls": this._buttonAccInfo && this._buttonAccInfo.ariaControls,
"ariaHaspopup": this._buttonAccInfo && this._buttonAccInfo.ariaHaspopup,
"title": this._buttonAccInfo && this._buttonAccInfo.title,
Expand Down
2 changes: 1 addition & 1 deletion packages/main/test/pages/Button.html
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
<ui5-icon name="invalid_name"></ui5-icon>
<ui5-button design="Emphasized" icon="invalid_name">Press</ui5-button>

<ui5-button id="button1" design="Emphasized">Action Bar Button</ui5-button>
<ui5-button id="button1" design="Emphasized" aria-expanded="true">Action Bar Button</ui5-button>
<ui5-button style="height: auto">Primary <br> button</ui5-button>
<ui5-button style="height: auto" design="Transparent">Secondary <span style="color:red;">button</span></ui5-button>
<ui5-button disabled id="button-disabled">Inactive</ui5-button>
Expand Down
17 changes: 16 additions & 1 deletion packages/main/test/specs/Button.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ describe("Button general interaction", () => {

button.setAttribute("icon", "add");
assert.strictEqual(button.shadow$$("ui5-icon").length, 1, "icon is present");

button.setAttribute("icon", "");
assert.strictEqual(button.shadow$$("ui5-icon").length, 0, "icon is not present");
});
Expand Down Expand Up @@ -78,4 +78,19 @@ describe("Button general interaction", () => {

assert.strictEqual(field.getProperty("value"), "6", "click should be called 6 times");
});

it("setting aria-expanded on the host is reflected on the button tag", () => {
const button = browser.$("#button1");
const innerButton = button.shadow$("button");

assert.strictEqual(innerButton.getAttribute("aria-expanded"), "true", "Attribute is reflected");

button.setAttribute("aria-expanded", "false");

assert.strictEqual(innerButton.getAttribute("aria-expanded"), "false", "Attribute is reflected");

button.removeAttribute("aria-expanded");

assert.strictEqual(innerButton.getAttribute("aria-expanded"), null, "Attribute is reflected");
})
});

0 comments on commit df9e4e9

Please sign in to comment.