-
Notifications
You must be signed in to change notification settings - Fork 101
feat(loading) - Update Spinner to SHINE styles; rename to Loading #2142
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
base: beta
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: afff724 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 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for stacks-svelte ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
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.
I would try to match what's in in the package that @abovedave did. A couple differences I'm noticing for the animation:
- Only 1 block is moving at a time, is it possible to have the second block start to go up before the first one comes down? To get more of a wave effect.
- In the package, each block goes up and down without stopping. In your version they seem to have a pause before going up again, it should be a more fluid up/down non-stop.
(This might be what you asked Dan's help for)
- The docs are overlapping for the button loader instead of scrolling.
|
When I was implementing the icon version, I was wondering if could just be used like a typical icon with the modifier |
dancormier
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.
@ttaylor-stack This PR seems somewhat removed from what's specified in the ticket. I've created a PR that points to this one to make these changes line up with the requested changes in SPARK-117.
Please merge it into your PR if you feel comfortable with that
SPARK-117
Figma
As mentioned in the ticket we will not be implementing the Logo Loader in this PR.
NOTES:
typeproperty in anticipation for the next part of the component. Is this the right choice or should we remove it until the the next part of the component is implemented? @giamir @dancormier