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

Dismissable alert close button padding #24478

Closed
wants to merge 1 commit into from
Closed

Dismissable alert close button padding #24478

wants to merge 1 commit into from

Conversation

MattJLeach
Copy link

@MattJLeach MattJLeach commented Oct 20, 2017

I have increased the y padding for the alert and made the padding for the close button the same all round.

Fixes #24469

@andresgalante
Copy link
Collaborator

Hi @MattJLeach Thanks a lot for trying to fix this.

The solution you are proposing adds a huge padding on the left or the alert. I don't think this is valid solution.

@MattJLeach
Copy link
Author

MattJLeach commented Oct 20, 2017

Would you prefer to have increased padding on the right and keep the left the same as it is now @andresgalante ? The only reason I increased both was to keep the text in the centre.

@andresgalante
Copy link
Collaborator

@MattJLeach I think the padding should go on the parent, not on the close icon, as I describe here #24469

@andresgalante
Copy link
Collaborator

Hi @MattJLeach, I am going to close this PR in favor of #24481

Thanks a lot for taking the time to make a contribution

@MattJLeach
Copy link
Author

Sounds good @andresgalante. Thanks for the feedback on my PR. I'll keep an eye out for any other issues I can help with but comment and discuss before doing an actual PR first.

@andresgalante
Copy link
Collaborator

Thanks man, feel free to send as many PRs as you can, it'll be my pleasure to review them.

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

Successfully merging this pull request may close these issues.

3 participants