feat(user card) - Update User Card to SHINE styles (part 2) #2123
Conversation
giamir
left a comment
giamir
left a comment
There was a problem hiding this comment.
Thanks Tavian for the PR. As you can see I am asking for few changes. Hopefully they are all understandable if not let me know and we can pair.
For the svelte component I suggest to create a UserCardAdditionalBling subcomponent so that we don't have to add a lot of logic and props in the existing root UserCard component but just a snippet.
Sidenote
I think AdditionalBling is a pretty poor name to describe the little extra icons that can be added to between the badges and the blings but it is what we have in Figma so I guess we can roll with it. I don't have a better name for now. cc @CGuindon
packages/stacks-classic/lib/components/user-card/user-card.less
Outdated
Show resolved
Hide resolved
| word-break: break-all; | ||
| } | ||
|
|
||
| & &--awarded-third { |
There was a problem hiding this comment.
How are those used? I cannot find them in the docs snippets?
There was a problem hiding this comment.
It's used to set the colors for the additional bling trophies. It's used in the awarded snippet like so
<div class="s-user-card--group s-user-card--awarded-third" title="…" data-controller="s-tooltip"> @Svg.Icon.AchievementsSm.With("native") </div>
There was a problem hiding this comment.
Thanks for clarifying. I would like to get @dancormier opinions on the API naming here. There is a part of me that still wonder if this "additional blings" are worthy to be included already in the central library or we should just have the consumer define them for now and only centralize when we see the pattern is used across several scenarios (not just collectives). What I mean is that instead of creating a s-user-card--awarded-third class to add just the color consumers could use atomic color classes and take care of it themselves (we could still keep an example in the docs but we won't have any dedicated less). The equivalent in the svelte component world would be just to have a snippet (e.g. additionalBlings - it would be nice to find a better name but it is what it is) to plug things in it but also there we will allow consumers to pass in it whatever they want.
Also cc @CGuindon for visibility.
There was a problem hiding this comment.
By the way what I am suggesting is some version of the last responsible moment. Do we have enough confidence to nail the API at this stage, with the information and the use case we have or is it better to keep things flexible for the time being and defer centralization of some styling to a later moment.
There was a problem hiding this comment.
I'm good with "just have the consumer define them for now" but I think we wanted @dancormier to weigh in as well
packages/stacks-classic/lib/components/user-card/user-card.less
Outdated
Show resolved
Hide resolved
packages/stacks-classic/lib/components/user-card/user-card.less
Outdated
Show resolved
Hide resolved
packages/stacks-svelte/src/components/UserCard/AvatarDeleted16.svelte
Outdated
Show resolved
Hide resolved
packages/stacks-svelte/src/components/UserCard/UserCardAdditionalBling.svelte
Outdated
Show resolved
Hide resolved
packages/stacks-svelte/src/components/UserCard/UserCardAdditionalBling.svelte
Outdated
Show resolved
Hide resolved
…m/StackExchange/Stacks into spark-127/update-user-card-part-2
| interface Props { | ||
| /** | ||
| * Account name for screen readers |
There was a problem hiding this comment.
NIT: wrong description. It's an internal only component, we could also skip jsdoc all together.
| width="72" | ||
| height="72" | ||
| preserveAspectRatio="none" | ||
| xlink:href="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAEgAAABICAYAAABV7bNHAAAACXBIWXMAACE4AAAhOAFFljFgAAAAAXNSR0IArs4c6QAAAARnQU1BAACxjwv8YQUAAAKZSURBVHgB7dxBbtpAFAbg92wv6I4jtDfIEWAT0UVbo4rQrBpOEOUEVU8Q9QRxV22gEo66oav6CO0NOALLLIJfPJEtHOTxMyZYIfyflMVocGx+yR4znmf+OQmFVqJPA79LBX5Mph0m/ksWyXZs60v2YbbrFPWJI6PTj/2gcJ/j6RkzX1Ex67Gm+xRbn5B0Twf9iCocq0NQCgEpvOQvyhrJKfTP9kHXo0V8t/rsRlj+k1jOwJjmts0chxYixfssO9ZUZOtwhRa2vtJjBQAA2Cts7pCzhhn+Tk762vD5oo3H06MlUztre/mfDzE/3Dt06YDFzJeMnxrVISAFAlIgIAUCUiAggJ2qPTN0/Nb3HVm2aUuz2e+grL/Xe3dGW3Pns1kYUQ0e1cSyPI8tE/EbCso6k31c0ZaYlhFRvdlQXKQVCEiBgBRe+qDsgXlSMBx8uKiy4auW27+9pa0v0hqH3DfUoOvJzWXyYPEoa5uLdCdrJB1UVRiG5tHJgnYsGX3m1KA0nE7WximmQEAKBKRAQAoEpPBEZJQ1HNn9qPTcyZ18JZe+EwAAwN5js9R21aK5bXnsobj+NfXjOL944fE65IhqTk2+FCJ8zoxf85UhIAUCUiAgBQJSVC5F2ITv++0mJvTzWi1apPPk21krRdhJUcJx732QDJifqUFMTvBndjOiJ4ZTTIGAFAhIUXt1RxmH4iC5KkTUKJ4TwLPDVaueDwWqnjeEgBQISIGAFAhIgYAUnnnRR9ZwsXiBHJGLfEkmAADA/uKqpQjmjQSm6N72j8pmAdaX9z86gFi+DYf9sKgvfW/al8LtlLKJ/PdaZ4Zy2xsmapcimHsDrlkftr68Py/2ShZMCr0mtrwcTi+b6Ng6yu5zUIqwIQSkuAe53NsA2WVtFwAAAABJRU5ErkJggg==" |
There was a problem hiding this comment.
@CGuindon this svg is actually embedding a png, is that on purpose? Didn't we have a regular svg?
There was a problem hiding this comment.
@ttaylor-stack Yes! Sorry that's a figma thing, I can't add an SVG to the figma component so I added the PNG for the design mockups. The SVG for the deleted user is here (also exported and attached)
There was a problem hiding this comment.
I don't think we need this file anymore @ttaylor-stack
CGuindon
left a comment
CGuindon
left a comment
There was a problem hiding this comment.
Is there an easy way to have the height of the container that the username is in be the same as the avatar height for both the Original poster and New contributor on the larger size. It's missing 1px of padding above and below I think to match.
SPARK-127
Figma
This PR covers part 2 of the User Card component which includes: