Skip to content
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

Remove padding in DialogContent #3427

Merged
merged 3 commits into from
May 23, 2024
Merged

Remove padding in DialogContent #3427

merged 3 commits into from
May 23, 2024

Conversation

JoanaMMoreira
Copy link
Contributor

@JoanaMMoreira JoanaMMoreira commented May 15, 2024

Copy link

changeset-bot bot commented May 15, 2024

🦋 Changeset detected

Latest commit: b2cc5a2

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@salt-ds/core Patch

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

Copy link

vercel bot commented May 15, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
saltdesignsystem ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 23, 2024 11:22am

@JoanaMMoreira JoanaMMoreira changed the title Fix spacing in DialogContent Remove padding in DialogContent May 15, 2024
@JoanaMMoreira JoanaMMoreira marked this pull request as ready for review May 15, 2024 14:28
@JoanaMMoreira JoanaMMoreira linked an issue May 15, 2024 that may be closed by this pull request
10 tasks
@joshwooding
Copy link
Contributor

Before:
image
image

After:
image
image

Without scrolling looks fine, but once there's a scrollbar it looks a little tight? Might not be an issue though

@origami-z
Copy link
Contributor

origami-z commented May 16, 2024

Without scrolling looks fine, but once there's a scrollbar it looks a little tight? Might not be an issue though

We probably need to live with this, unless there's a way to add padding only when scroll bar appears

@JoanaMMoreira
Copy link
Contributor Author

Without scrolling looks fine, but once there's a scrollbar it looks a little tight? Might not be an issue though

We probably need to live with this, unless there's a way to add padding only when scroll bar appears

I don't think there's a nice way of adding padding for the scroll bar, there are some hacks like adding a transparent thicker border to it but it's not really ideal

Copy link
Contributor

@origami-z origami-z left a comment

Choose a reason for hiding this comment

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

Can we update QA stories, so that right spacing change could be captured in Chromatic

@JoanaMMoreira
Copy link
Contributor Author

Can we update QA stories, so that right spacing change could be captured in Chromatic

Just added a QA story 👍

@origami-z
Copy link
Contributor

@navkaur76 does any of padding removal impact pref dialog pattern?

@JoanaMMoreira JoanaMMoreira merged commit 33ffe0e into main May 23, 2024
15 checks passed
@JoanaMMoreira JoanaMMoreira deleted the fix-dialog-spacing branch May 23, 2024 11:43
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.

Dialog spacing is not on spec
3 participants