Skip to content

feat(ui5-avatar): implement accessibility spec #1484

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.

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 3 commits into from
Apr 22, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions packages/main/src/Avatar.hbs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
<div class="ui5-avatar-root">
{{#if image}}
<span alt="avatar" class="ui5-avatar-img" style="background-image: url({{image}})"></span>
<span class="ui5-avatar-img" style="background-image: url({{image}})" role="img" aria-label="{{accessibleNameText}}"></span>
{{else if icon}}
<ui5-icon class="ui5-avatar-icon" name="{{icon}}"></ui5-icon>
<ui5-icon class="ui5-avatar-icon" name="{{icon}}" accessible-name="{{accessibleNameText}}"></ui5-icon>
{{else if initials}}
<span class="ui5-avatar-initials">{{validInitials}}</span>
{{/if}}
Expand Down
34 changes: 33 additions & 1 deletion packages/main/src/Avatar.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
import UI5Element from "@ui5/webcomponents-base/dist/UI5Element.js";
import litRender from "@ui5/webcomponents-base/dist/renderer/LitRenderer.js";
import { fetchI18nBundle, getI18nBundle } from "@ui5/webcomponents-base/dist/i18nBundle.js";

// Template
import AvatarTemplate from "./generated/templates/AvatarTemplate.lit.js";

import { AVATAR_TOOLTIP } from "./generated/i18n/i18n-defaults.js";

// Styles
import AvatarCss from "./generated/themes/Avatar.css.js";

Expand Down Expand Up @@ -142,6 +145,19 @@ const metadata = {
type: String,
defaultValue: AvatarBackgroundColor.Accent6,
},

/**
* Defines the text alternative of the <code>ui5-avatar</code>.
* If not provided a default text alternative will be set, if present.
*
* @type {string}
* @defaultvalue ""
* @public
* @since 1.0.0-rc.7
*/
accessibleName: {
type: String,
},
},
slots: /** @lends sap.ui.webcomponents.main.Avatar.prototype */ {
},
Expand Down Expand Up @@ -173,6 +189,11 @@ const metadata = {
* @public
*/
class Avatar extends UI5Element {
constructor() {
super();
this.i18nBundle = getI18nBundle("@ui5/webcomponents");
}

static get metadata() {
return metadata;
}
Expand All @@ -190,7 +211,10 @@ class Avatar extends UI5Element {
}

static async onDefine() {
await Icon.define();
await Promise.all([
fetchI18nBundle("@ui5/webcomponents"),
Icon.define(),
]);
}

get validInitials() {
Expand All @@ -202,6 +226,14 @@ class Avatar extends UI5Element {

return null;
}

get accessibleNameText() {
if (this.accessibleName) {
return this.accessibleName;
}

return this.i18nBundle.getText(AVATAR_TOOLTIP) || undefined;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The || undefined part seems redundant.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If getText function return "" empty aria-label attribute will be set.

Should I remove it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is ok, otherwise the attribute will be set with value empty string.

}
}

Avatar.define();
Expand Down