Avatar: add alt and src

[siddharthkp]

Describe your changes here.

Closes #1405

Screenshots

before:

after:

Worth mentioning

I've made alt text optional based on @alliethu's comment. We set the default value to "" (empty string) so that screen readers ignore it.

Room for improvement

1. The default value for alt shows up as - even though the value is an empty string. This is because we set savePropValueAsString which treats undefined and "" the same. It would be interesting to show an explicit empty string as null and empty string have different meaning for alt (reference via @alliethu, thx! 👋 )
2. Right now, we don't indicate a prop is required, it would be really nice to have that.
3. The order of props in the documentation is not the same as the order they are defined. It would be nice to control the order

Merge checklist

• Added/updated tests
• Added/updated documentation
• Tested in Chrome
• Tested in Firefox
• Tested in Safari
• Tested in Edge

Take a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 9ab233c

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[github-actions]

size-limit report 📦

Path	Size
dist/browser.esm.js	48.59 KB (0%)
dist/browser.umd.js	48.85 KB (0%)

[alliethu]

The default value for alt shows up as - even though the value is an empty string.

@siddharthkp Is this alt update in a place I can access it and run through it with a ScreenReader? I'm a little concerned the output value being -, might not be accessible to SR users.

[siddharthkp]

@siddharthkp Is this alt update in a place I can access it and run through it with a ScreenReader? I'm a little concerned the output value being -, might not be accessible to SR users.

Hi @alliethu!

Whoops, bad choice of words on my side! The rendered alt text is still an empty string by default (didn't change that). However, we show the wrong value in the documentation (- instead of empty string)

The preview is deployed here: preview/Avatar

The rendered alt text is an empty string:

[alliethu]

@siddharthkp Perfect! Thank you for clarifying!! 🙇🏼‍♀️

[colebemis]

Looks great. Thanks for doing this, @siddharthkp! ✨

I agree with all of the things you listed in Room for improvement. Do you want to open up PRs for those next?

[siddharthkp]

I agree with all of the things you listed in Room for improvement. Do you want to open up PRs for those next?

Sounds good 👍

What's our merge process, do I just merge this when the build passes?

[colebemis]

What's our merge process, do I just merge this when the build passes?

Yeah, go ahead and merge it 🚢