-
Notifications
You must be signed in to change notification settings - Fork 830
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
Better select styles for Firefox and Safari #128
Better select styles for Firefox and Safari #128
Conversation
…n Firefox and Safari (to be applied to same div as .slds-form-element__control)
…styles in Firefox and Safari (to be applied to same div as .slds-form-element__control)" This reverts commit d14b9df.
…n Firefox and Safari (to be applied to the same div as .slds-form-element__control)
By analyzing the blame information on this pull request, we identified @aputinski and @brandonferrua to be potential reviewers |
To clarify, this is for #122 My signed agreement has also just been sent. |
Thank you for your contribution and signed agreement! We will review this PR, and we look forward to integrating your improvements! |
@@ -22,3 +22,35 @@ | |||
} | |||
} | |||
} | |||
|
|||
#{$css-prefix}form-element__select-container { |
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.
forgot the .
also, to align with our naming patterns, can you change the class to .#{$css-prefix}select__container
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.
D'oh! Yeah I probably need those .
s haha.
For the second comment, wouldn't __
technically mean the container would be a child of the .slds-select
class? This wouldn't be the case, as the container would wrap the select itself. Or did you perhaps mean .slds-form-element__container
? Please advise.
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.
You are correct when it comes to standard BEM but we have some special rules in place that make this ok. We'll be releasing these patterns to the contributing guidelines shortly.
Thank you for the adjustments, @kevinberonilla. We'll do a round of browser testing soon, and then we can merge this. |
@@ -21,4 +21,39 @@ | |||
padding: $spacing-x-small; | |||
} | |||
} | |||
} | |||
|
|||
&__container { |
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.
Thanks for the changes! I hate to do this, but can you change this to a single underscore? _
.
Once those changes are made, we can merge this! thanks again! |
Better select styles for Firefox and Safari
This is my suggested fix to overriding any
<select>
styles enforced by the browser. The div with.slds-form-element__control
can also have.slds-form-element__select-container
to set the select's appearance to none, and add proper padding and pseudo elements for create the drop-down arrows.