-
Notifications
You must be signed in to change notification settings - Fork 101
feat(user card) - Update User Card to SHINE styles (part 2) #2123
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
Conversation
| interface Props { | ||
| /** | ||
| * Account name for screen readers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIT: wrong description. It's an internal only component, we could also skip jsdoc all together.
packages/stacks-svelte/src/components/UserCard/AvatarDeleted.svelte
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need this file anymore @ttaylor-stack
CGuindon
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@CGuindon updating original poster is fine I can do that now however with the new contributor in the figma it matches the height of the other user badges not the avatar. Do we still want to increase the height to match the avatars? Also if that's the case do we want to update all user badges to match the height of the avatar? |
packages/stacks-svelte/src/components/UserCard/AvatarDeleted.svelte
Outdated
Show resolved
Hide resolved
…m/StackExchange/Stacks into spark-127/update-user-card-part-2
No my mistake, just original poster is fine. |
|
|
||
| <li> | ||
| <Badge text={type} type="user" userType={type} size="sm" /> | ||
| {#if tooltip && type === "new"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ttaylor-stack NIT, no need here to check also for type ==="new". This can be left generic. If tooltip is defined we show the tooltip otherwise we don't.
| @@ -1,12 +1,17 @@ | |||
| <script lang="ts" module> | |||
| export type Size = "sm" | "lg" | undefined; | |||
| export type AwardBlings = "first" | "second" | "third" | undefined; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ttaylor-stack Leftover?
giamir
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have left a couple of minor comments. Once those are addressed I think this is good enough to be merged. Thanks @ttaylor-stack


SPARK-127
Figma
This PR covers part 2 of the User Card component which includes: