Skip to content

Conversation

@mukunku
Copy link
Collaborator

@mukunku mukunku commented Jan 28, 2026

Summary

This PR removes a few of the existing sidebar widget functionality while salvaging what I could and updating it according to the new SHINE designs.

A lot of the old variations might not be relevant anymore so the idea is to add them back on a case-by-case basis as needed. The reason they were removed was because it didn't make sense to spend effort on them without proper designs.

How to Test

Check out the Sidebar Widgets page: https://deploy-preview-2155--stacks.netlify.app/product/components/sidebar-widgets/

@changeset-bot
Copy link

changeset-bot bot commented Jan 28, 2026

🦋 Changeset detected

Latest commit: 38625af

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

@mukunku mukunku changed the base branch from develop to beta January 28, 2026 22:13
@netlify
Copy link

netlify bot commented Jan 28, 2026

Deploy Preview for stacks ready!

Name Link
🔨 Latest commit 38625af
🔍 Latest deploy log https://app.netlify.com/projects/stacks/deploys/697d0db7da748400086a5629
😎 Deploy Preview https://deploy-preview-2155--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 28, 2026

Deploy Preview for stacks-svelte ready!

Name Link
🔨 Latest commit 38625af
🔍 Latest deploy log https://app.netlify.com/projects/stacks-svelte/deploys/697d0db781ca210008d1d40a
😎 Deploy Preview https://deploy-preview-2155--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.

@mukunku mukunku marked this pull request as ready for review January 28, 2026 23:20
Copy link
Contributor

@giamir giamir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @mukunku

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.

  1. The sidebar is 344px without any padding so the sidebar widgets shouldn't have left/right padding. That spacing will be handled on the outside of the whole right sidebar.
Image
  1. Add s-btn s-btn__xs s-btn__clear to the s-sidebarwidget--action (the + track button) element so that it gets the button stylings for height and hover states.

  2. Then you're padding between the header and middle container can be a normal 12px class (instead of 16 + 2, because the button will make the header container taller).

(screenshot shows hover state)

Image
  1. Should the text inside s-sidebarwidget--content be in a

    element? (not 100% but wondering about a11y or html standards with text if it's needed here or not)


  1. Title: There's a space before "Community Achievements" that shouldn't be there (made me think there was extra padding) and the title should be fw-medium (instead of bold).

  2. (more rare use case but thinking of future proofing) Can we have the icon be it's own "column" on the left in the header? So that if the title needs to wrap to a second line, the text wouldn't wrap under the icon.

  3. AND make sure there's at least 6px padding in between the title and the button on the right (so if the title does get long, it won't touch the button's edge).

Screenshot 2026-01-30 at 8 56 13 AM Screenshot 2026-01-30 at 8 56 24 AM

Popped that second example in the Figma


  1. I feel like the warning note should be above the example not after (easier to miss) — that means 2 notices before, I think that's fine.
Screenshot 2026-01-30 at 8 51 46 AM

@mukunku
Copy link
Collaborator Author

mukunku commented Jan 30, 2026

Thanks for the review @CGuindon ! I believe I addressed all your comments:

  1. I removed the x padding so we get the full 344px width now
  2. I updated the header action style 👍🏾
  3. I played around with the paddings to match what you have in the figma. Please let me know what you think.
  4. It doesn't have to be; it's valid html and accessible to screen readers. It's just not semantically correct, and is more proper for it to be wrapped with something. I wrapped it in <span> now just because...
  5. Removed the space and set the font-weight to 500
  6. I made the icon its own column but there's no way to top align the icon and header cta if the title ends up being multiple lines. Meaning the icon and header cta will be centered always. Still looks okay with 2 line headers I think.
  7. Added 6px padding 👍🏾
  8. Moved the warning above now.

@mukunku mukunku requested a review from CGuindon January 30, 2026 19:44
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.

There's no Svelte component right? Should there be? or is this a different ticket? Approving the Stacks CLassic version so you can merge

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.

4 participants