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

🐛 Copy size does not scale in Avatar fallback #694

Closed
LaurentDario opened this issue Jun 28, 2021 · 9 comments
Closed

🐛 Copy size does not scale in Avatar fallback #694

LaurentDario opened this issue Jun 28, 2021 · 9 comments
Assignees
Labels

Comments

@LaurentDario
Copy link
Collaborator

All Avatar fallback seem to share the same 14px font-size.

Describe the bug

The font-size of the Avatar text fallback doesn't scale up from the medium size (14px).
It is scaling from 2xs to md.

Steps to reproduce

Reproducible in the documentation
image

(Notice that the size documentation seems to react strangely to all the SRC being empty, the first 3 avatars are not showing)

Expected results

According to Figma, the fonts should be as follow:

  • xs: 10px
  • sm: 12px
  • md: 14px
  • lg: 16px
  • xl: 18px
  • 2xl: 22px
@patricklafrance
Copy link
Member

patricklafrance commented Jun 28, 2021

Thanks for the issue. Definitely have something to do the an empty src because with a src it does work, have a look at the following Chromatic test: https://sg-storybook.netlify.app/?path=/story/chromatic-avatar--initials

What your use case which result in having empty src ?

@LaurentDario
Copy link
Collaborator Author

The empty src were just to test the fallback, in cases where the src is invalid.
Just realized that the behavior of the font-size is different if the SRC is simply removed:
image

In Apricot it would be the case for an Avatar with an invalid SRC:
image

@LaurentDario
Copy link
Collaborator Author

Also wondering if that notion of and not being part of the initials would make sense to be addressed in Orbit or if it's something we should work on (if needed) in Apricot directly, so it shows GS in that case instead of Ga.

@patricklafrance
Copy link
Member

patricklafrance commented Jun 28, 2021

The empty src were just to test the fallback, in cases where the src is invalid.
Just realized that the behavior of the font-size is different if the SRC is simply removed:
image

In Apricot it would be the case for an Avatar with an invalid SRC:
image

For the Avatar component, an empty src is not the same as an invalid src. When the underlying source is invalid, the native Image object will fire an error event which will be catched by the component and render an initials avatar as fallback.

@patricklafrance
Copy link
Member

Also wondering if that notion of and not being part of the initials would make sense to be addressed in Orbit or if it's something we should work on (if needed) in Apricot directly, so it shows GS in that case instead of Ga.

Could you ellaborate on this please, I am not sure I fully understand what you mean.

@LaurentDario
Copy link
Collaborator Author

I don't think it belongs into the Orbit realm.
The initials used as placeholder (CH in the example of Chris Hadfield) is built off the name prop. This works well for users, but it doesn't always work for group names and teams names, where you could have all sorts of names like Greenfelder and Sons or Group-with-a-long-name.

In the example of Greenfelder and Sons, the expected initials should be GS instead of Ga. That means adding a logic to maybe ignore and in the way the initials are picked up.
That doesn't need to be addressed here, just opening the door to discussions. I realize that this can get tricky for other names we have in our Fake APIs.

To summarize this, here's a screenshot of the demo with some examples:
image

1st Avatar: Control, everything is fine
2nd Avatar: A name with an and
3rd Avatar: A name with a whole lot of words!
4th Avatar: A name with a dash
5th Avatar: The regular Avatar but with an invalid SRC, with its Placeholder with the wrong size.

@patricklafrance
Copy link
Member

patricklafrance commented Jun 28, 2021

Oh, sorry I miss the fact that your example was for a group, thanks for the clarification.

Yes indeed, it's currently out of Orbit realm. We could eventually add some kind of GroupAvatar component, it's probably better if you keep it in Apricot for now.

Could be a great request :)

@LaurentDario
Copy link
Collaborator Author

Hopefully I gave enough context in the Request: #695
Something that isn't ideal for now with handling it on the Apricot side is that the name we pass down to pick the Initials is also the name used for the aria-label of the avatar.
This means that if we add a layer of regex to exclude, for example, and, the aria-label for the avatar of Jast and Sons will become Jast Sons.

@patricklafrance
Copy link
Member

Hopefully I gave enough context in the Request: #695
Something that isn't ideal for now with handling it on the Apricot side is that the name we pass down to pick the Initials is also the name used for the aria-label of the avatar.
This means that if we add a layer of regex to exclude, for example, and, the aria-label for the avatar of Jast and Sons will become Jast Sons.

Sorry for the late reply, you can always provide an aria-label prop which will superceed the name prop but it's currently rendered on the div container instead of the img itself, I will fix that.

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

No branches or pull requests

2 participants