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

Add avatar component #689

Open
wants to merge 32 commits into
base: release/1.1.0
Choose a base branch
from
Open

Add avatar component #689

wants to merge 32 commits into from

Conversation

cg-julian-taylor
Copy link
Collaborator

No description provided.

@cg-julian-taylor cg-julian-taylor changed the base branch from main to release/1.1.0 September 6, 2024 15:37
@alexwbbr
Copy link
Collaborator

alexwbbr commented Sep 10, 2024

@cg-julian-taylor please can you alter the without style avatar documentation to use the text instead of the link. I also think when using the external link you shouldn't see text
Screenshot 2024-09-10 at 09 33 11
The link should just accept a url and maybe some aria labels, when using the external link you should be able to see the image or text

Please can you also make sure the avatar component is at the top of the documentation to keep it in alphabetical order

@cg-julian-taylor
Copy link
Collaborator Author

No problem with changing the without style documentation. It does look a bit odd.

As for what you pass in to the avatar itself I believe you should be able to pass in an aria-label as a prop.
You can pass in whatever child components you wish, e.g. an SVG.

I assume the typical use case of passing an image url or initials, however I wanted to demonstrate that the contents are flexible so used an external link as that was the most likely use case I could think of adding.

I also made the assumption that if a user is injecting something such as an SVG they would manipulate styling.

However if I am wrong or you disagree please let me know what use cases I should aim for.

src/avatar/Avatar.tsx Outdated Show resolved Hide resolved
@alexwbbr
Copy link
Collaborator

@cg-julian-taylor can you please remove the design system folder from storybook as it is currently broken. It will be picked up in a separate ticket as it requires a bit more work.

src/avatar/Avatar.tsx Outdated Show resolved Hide resolved
src/avatar/Avatar.tsx Outdated Show resolved Hide resolved
src/avatar/Avatar.tsx Outdated Show resolved Hide resolved
src/avatar/Avatar.tsx Outdated Show resolved Hide resolved
src/avatar/Avatar.tsx Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Dec 3, 2024

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (6d567f6) to head (4a54de2).

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@               Coverage Diff               @@
##           release/1.1.0      #689   +/-   ##
===============================================
  Coverage         100.00%   100.00%           
===============================================
  Files                 89        90    +1     
  Lines               1663      1678   +15     
  Branches             584       591    +7     
===============================================
+ Hits                1663      1678   +15     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants