Skip to content
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

feat(avatar): add label prop for alternative text & aria-label #6910

Merged
merged 7 commits into from May 4, 2023

Conversation

anveshmekala
Copy link
Contributor

@anveshmekala anveshmekala commented May 3, 2023

Related Issue: #5564

Summary

This PR will add a label property in calcite-avatar which is used to provide context for Assistive Technology users.

@github-actions github-actions bot added the enhancement Issues tied to a new feature or request. label May 3, 2023
@anveshmekala anveshmekala marked this pull request as ready for review May 3, 2023 20:06
@anveshmekala anveshmekala requested a review from a team as a code owner May 3, 2023 20:06
@anveshmekala anveshmekala added the pr ready for visual snapshots Adding this label will run visual snapshot testing. label May 3, 2023
Copy link
Member

@geospatialem geospatialem left a comment

Choose a reason for hiding this comment

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

Thanks for taking a look so fast! 🐎 A few recommendations below for support of aria-label and doc considerations.

@@ -77,7 +80,7 @@ export class Avatar {
const initials = this.generateInitials();
const backgroundColor = this.generateFillColor();
return (
<span class="background" style={{ backgroundColor }}>
<span aria-label={this.label || this.fullName} class="background" style={{ backgroundColor }}>
Copy link
Member

Choose a reason for hiding this comment

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

To ensure the aria-label is supported, we'll need to add a role to the span element. It seems like the figure role is the best fit.

@@ -39,6 +39,9 @@ export class Avatar {
/** Specifies the unique id of the user. */
@Prop({ reflect: true }) userId: string;

/** When `thumbnail` is defined, specifies alternate text for the image. Otherwise specifies accessible label for the component.*/
Copy link
Member

Choose a reason for hiding this comment

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

Since this could also apply to a non-image, how about the following?:

Accessible name for the component. Specifies alternative text when thumbnail is defined, otherwise specifies an accessible label.

Copy link
Member

Choose a reason for hiding this comment

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

For line 33, can it reflect the support for non-images?

Specifies the full name of the user. When label and thumbnail are not defined, specifies the accessible name for the component.

@anveshmekala anveshmekala added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels May 4, 2023
Copy link
Member

@geospatialem geospatialem left a comment

Choose a reason for hiding this comment

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

🏆 Sounds great in JAWS 🦈 and NVDA 🟣

Copy link
Member

@jcfranco jcfranco left a comment

Choose a reason for hiding this comment

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

👀👀👀👀👀👀👀👀👀👀👀👀👀👀👀👀👀👀👀👀👀👀👀
👀🖥👀👀👀🖥👀🖥🖥🖥👀👀🖥🖥🖥👀🖥🖥🖥🖥👀🖥👀
👀🖥🖥👀👀🖥👀👀🖥👀👀🖥👀👀👀👀🖥👀👀👀👀🖥👀
👀🖥👀🖥👀🖥👀👀🖥👀👀🖥👀👀👀👀🖥🖥🖥👀👀🖥👀
👀🖥👀👀🖥🖥👀👀🖥👀👀🖥👀👀👀👀🖥👀👀👀👀👀👀
👀🖥👀👀👀🖥👀🖥🖥🖥👀👀🖥🖥🖥👀🖥🖥🖥🖥👀🖥👀
👀👀👀👀👀👀👀👀👀👀👀👀👀👀👀👀👀👀👀👀👀👀👀

@anveshmekala anveshmekala merged commit e8d78e7 into master May 4, 2023
12 checks passed
@anveshmekala anveshmekala deleted the anveshmekala/5564-add-alt-text-for-avatar branch May 4, 2023 22:37
@github-actions github-actions bot added this to the 2023 May Priorities milestone May 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Issues tied to a new feature or request. pr ready for visual snapshots Adding this label will run visual snapshot testing.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants