-
Notifications
You must be signed in to change notification settings - Fork 99
feat(activity-indicator): update to new visual language #2027
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
Add ActivityIndicator to Stacks Svelte
🦋 Changeset detectedLatest commit: cfd00d4 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for stacks-svelte ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for stacks ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
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.
Everything looks good here except the whole vertical text alignment issue you mentioned. I made a suggestion to use inline-flex which will give us a better balance (not perfect but hopefully acceptable). Looks like this is what some other libraries (MUI, Ant Design) handle it as well.
packages/stacks-classic/lib/components/activity-indicator/activity-indicator.less
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.
Ok don't hate me but... seeing this in the browser vs figma (using a different system font as well) makes them look extra small. I think we re-used the old component that was in Figma and updated the colors but that didn't match the sizing that was in stacks. Looking from old to new it does look to tight/small.
I've bump up the sizing on the dot for both:
- large size with characters would be 16x16 (roughly the same size as current)
- small size would be 10x10 (in between what we have today and what I had originally proposed)
Both updated in Figma as well.
@CGuindon I have applied your requested changes here. Can you give it another review? Thanks |
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.
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.
|
@dancormier Merging this as agreed in our conversation early. If you have a minute feel free to play around with the indicator on the notification icon. Thanks a lot ❤️ |



SPARK-62
Figma
Here is what changed:
.s-activity-indicator__smvariant (10x10). (docs)ActivityIndicatorcomponent. (storybook)Note 1: please ignore for now the activity indicator on the avatar not being rendered correctly. It will be sorted in the avatar PR that is coming right after this one.

Note 2: Topbar screenshots are failing because they are using activity indicators which have changed slightly (10x10 instead of 12x12)