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

feat(dialog): 8px margin between dialog action buttons #5283

Closed
wants to merge 2 commits into from

Conversation

karlhaas
Copy link

According to https://material.io/guidelines/components/dialogs.html#dialogs-specs there should be a margin of 8px between action buttons.

@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Jun 21, 2017
@karlhaas karlhaas changed the title 8px margin between dialog action buttons feat(dialog): 8px margin between dialog action buttons Jun 22, 2017
@jelbourn
Copy link
Member

@crisbeto thoughts? Maybe this case is specific enough that the margins make sense, but I could still see it being intrusive in some cases

@crisbeto
Copy link
Member

I think that they make sense, but they'll look off in RTL @karlhaas.

@karlhaas
Copy link
Author

karlhaas commented Jul 13, 2017

Is there a way to find out (in scss) if rtl is used? Something like fxLayoutGap could work well. We use fxLayoutGap in our projet to solve it but i thought it should work out of the box ...

@crisbeto
Copy link
Member

You can use the [dir="rtl"] in the selector.

@karlhaas
Copy link
Author

@crisbeto it is always just a margin between two action buttons. in my opinion it makes no difference ...

@crisbeto
Copy link
Member

It does make a difference. When it is in RTL or align="end" is set, the margin needs to be flipped, otherwise it'll stack with the dialog padding. Another issue that this PR introduces is that it adds a margin to the inner content of the buttons (note the "Close" button), which throws off the alignment. Here's a screenshot that shows both issues:

kmwjnsc

@karlhaas
Copy link
Author

thx @crisbeto for the explanation

@karlhaas karlhaas closed this Jul 13, 2017
crisbeto added a commit to crisbeto/material2 that referenced this pull request Jul 15, 2017
Based on the spec, buttons inside the dialog actions are supposed to have a margin. This is follow-up from angular#5283.
crisbeto added a commit to crisbeto/material2 that referenced this pull request Jul 17, 2017
Based on the spec, buttons inside the dialog actions are supposed to have a margin. This is follow-up from angular#5283.
kara pushed a commit that referenced this pull request Jul 19, 2017
Based on the spec, buttons inside the dialog actions are supposed to have a margin. This is follow-up from #5283.
kara pushed a commit that referenced this pull request Jul 20, 2017
Based on the spec, buttons inside the dialog actions are supposed to have a margin. This is follow-up from #5283.
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes PR author has agreed to Google's Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants