-
Notifications
You must be signed in to change notification settings - Fork 467
Fixing inconvenient styles for contextual menu. #289
Conversation
Hi @s-KaiNet, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution! The agreement was validated by Microsoft and real humans are currently evaluating your PR. TTYL, MSBOT; |
@@ -8,19 +8,20 @@ | |||
|
|||
.ms-ContextualMenu { | |||
.ms-font-m; | |||
.ms-u-normalize; | |||
.resetMargins; | |||
.resetPadding; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you find another bug that required removing ms-u-normalize
and adding resetMargins
and resetPadding
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.ms-u-normalize
contains .resetBoxShadow
which is not desired, because menu has .drop-shadow
. But now I see that I missed .ms-u-borderBox
to copy from .ms-u-normalize
. So .ms-ContextualMenu
should contains all styles from .ms-u-normalize
except .resetBoxShadow
. Sorry for missing .ms-u-borderBox
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No worries, would you mind adding in .ms-u-borderBox
back in and committing it? It makes sense to take out .resetBoxShadow
so we should be able to merge this in once border-box is set.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
Looks good to me. We just need one more reviewer from our team to take a look and we can merge it in. Thank you! |
Looks great! Excited to merge this in. |
Fixing inconvenient styles for contextual menu.
Solves #287