Skip to content

Conversation

@ttaylor-stack
Copy link
Collaborator

@ttaylor-stack ttaylor-stack commented Jan 23, 2026

SPARK-117

Figma

As mentioned in the ticket we will not be implementing the Logo Loader in this PR.

NOTES:

  • The animation matches what is in the Figma however I think it slightly differs from the icon in the icons package. @CGuindon can you confirm if that's alright?
  • I wasn't sure on exact timings of the animation so I tried to match what was in the beta package. cc: @CGuindon
  • I've added in a type property 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
  • For some reason when the loader is a child of a button element the animation stopped working correctly so I created a second animation for that. There is probably a better way to do this. So I'm happy to hear any suggestions @dancormier @giamir

@changeset-bot
Copy link

changeset-bot bot commented Jan 23, 2026

🦋 Changeset detected

Latest 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

@netlify
Copy link

netlify bot commented Jan 23, 2026

Deploy Preview for stacks ready!

Name Link
🔨 Latest commit afff724
🔍 Latest deploy log https://app.netlify.com/projects/stacks/deploys/6978ec1cf108fe0008ad0685
😎 Deploy Preview https://deploy-preview-2142--stacks.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@netlify
Copy link

netlify bot commented Jan 23, 2026

Deploy Preview for stacks-svelte ready!

Name Link
🔨 Latest commit afff724
🔍 Latest deploy log https://app.netlify.com/projects/stacks-svelte/deploys/6978ec1c1f49cb00078abb7b
😎 Deploy Preview https://deploy-preview-2142--stacks-svelte.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@ttaylor-stack ttaylor-stack marked this pull request as ready for review January 26, 2026 12:53
Copy link
Collaborator

@CGuindon CGuindon left a 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.
Screenshot 2026-01-26 at 7 28 49 AM

@abovedave
Copy link
Collaborator

When I was implementing the icon version, I was wondering if could just be used like a typical icon with the modifier s-btn__icon which could also be resizable. Might be a chance to remove some code from the css bundle.

@dancormier dancormier changed the title feat(loader) - Add Loader component and remove Spinner component feat(loading) - Add Loader component and remove Spinner component Jan 27, 2026
@dancormier dancormier changed the title feat(loading) - Add Loader component and remove Spinner component feat(loading) - Update Spinner to SHINE styles; rename to Loading Jan 27, 2026
Copy link
Contributor

@dancormier dancormier left a 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants