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

fix(modal): replace width rule with padding #3607

Merged

Conversation

emyarod
Copy link
Member

@emyarod emyarod commented Jul 31, 2019

Closes #3412

This PR replaces the width rule on modal header and content with a padding change instead

Testing / Reviewing

Ensure the modal appears correct

@netlify
Copy link

netlify bot commented Jul 31, 2019

Deploy preview for the-carbon-components ready!

Built with commit 0913bf9

https://deploy-preview-3607--the-carbon-components.netlify.com

@netlify
Copy link

netlify bot commented Jul 31, 2019

Deploy preview for carbon-components-react ready!

Built with commit 0913bf9

https://deploy-preview-3607--carbon-components-react.netlify.com

@netlify
Copy link

netlify bot commented Jul 31, 2019

Deploy preview for carbon-elements ready!

Built with commit 0913bf9

https://deploy-preview-3607--carbon-elements.netlify.com

Copy link
Contributor

@asudoh asudoh left a comment

Choose a reason for hiding this comment

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

LGTM 👍 - Thanks @emyarod!

@asudoh asudoh requested a review from a team July 31, 2019 23:55
@ghost ghost requested review from aagonzales and removed request for a team July 31, 2019 23:55
@elizabethsjudd
Copy link
Contributor

elizabethsjudd commented Aug 1, 2019

Not that I disagree with this change but I'm curious, isn't this a breaking change for any user of Carbon 10 that has asked how to overwrite the 75% width setup for modal (i've seen it multiple times in slack and the issue backlog)?

Copy link
Member

@aagonzales aagonzales left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Collaborator

@tw15egan tw15egan left a comment

Choose a reason for hiding this comment

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

👍

@tw15egan tw15egan merged commit 1288a0f into carbon-design-system:master Aug 1, 2019
@asudoh
Copy link
Contributor

asudoh commented Aug 1, 2019

@elizabethsjudd I don't think it's likely, but let us know if you can think of any specific case.

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

Successfully merging this pull request may close these issues.

Modal body overflows unexpectedly and scroll bar is not in container
5 participants