contentful / forma-36

A design system by Contentful

https://f36.contentful.com

MIT License

331 stars 81 forks source link

feat: update Avatar and Navbar.Account to display initials if avatar is not available [AHOY-3140] [AHOY-3146] #2757

Closed budimirbudimir closed 3 months ago

budimirbudimir commented 3 months ago

Purpose of PR

Updating Avatar component and updating its dependent Navbar.Account component to contain the initials fallback option, for rendering initials if avatar URL is absent.
If neither - avatar URL not initials - are present, display fallback option of grey background.

PR Checklist

[x] I have read the relevant readme.md file(s)
[x] All commits follow our Git commit message convention
[x] Tests are added/updated/not required
[x] Tests are passing
[x] Storybook stories are added/updated/not required
[x] Usage notes are added/updated/not required
[x] Has been tested based on Contentful's browser support
[x] Doesn't contain any sensitive information

changeset-bot[bot] commented 3 months ago

⚠️ No Changeset found

Latest commit: 24d7729a96c77c0faff2117a4b019e1bb0339c66

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.

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

💥 An error occurred when fetching the changed packages and changesets in this PR

``` Some errors occurred when validating the changesets config: The package "@contentful/f36-components" depends on the ignored package "@contentful/f36-navbar", but "@contentful/f36-components" is not being ignored. Please add "@contentful/f36-components" to the `ignore` option. ```

vercel[bot] commented 3 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
forma-36	✅ Ready (Inspect)	Visit Preview	May 22, 2024 2:56pm

github-actions[bot] commented 3 months ago

size-limit report 📦

Path	Size	Loading time (3g)	Running time (snapdragon)	Total time
CommonJS	134.54 KB (+1.24% 🔺)	2.7 s (+1.24% 🔺)	553 ms (+2.96% 🔺)	3.3 s
Module	131.13 KB (+1.2% 🔺)	2.7 s (+1.2% 🔺)	833 ms (+35.96% 🔺)	3.5 s

Annmary12 commented 3 months ago

Code looks good to me but I wonder if we want to do some sort of handling of strings that are too long? Slice them after 2 or 3 characters maybe?

Hi @denkristoffer, I don't believe this will be necessary since the strings consist of two-character initials.

Side note: I'm taking over from Budimir as he is on vacation.

denkristoffer commented 3 months ago

Code looks good to me but I wonder if we want to do some sort of handling of strings that are too long? Slice them after 2 or 3 characters maybe?

Hi @denkristoffer, I don't believe this will be necessary since the strings consist of two-character initials.

Side note: I'm taking over from Budimir as he is on vacation.

The thing we have to remember is that the design system is also used outside of our own product, so I'd prefer to handle this case so customers can't break the component. I'd either slice the string or change the type to only take two characters.

Annmary12 commented 3 months ago

Code looks good to me but I wonder if we want to do some sort of handling of strings that are too long? Slice them after 2 or 3 characters maybe?

Hi @denkristoffer, I don't believe this will be necessary since the strings consist of two-character initials. Side note: I'm taking over from Budimir as he is on vacation.

The thing we have to remember is that the design system is also used outside of our own product, so I'd prefer to handle this case so customers can't break the component. I'd either slice the string or change the type to only take two characters.

I see your point, that makes sense. I will add that now