-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
TextField & SearchBox: Specified placeholder colors #3762
Conversation
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.
Looks good here.
@@ -11,7 +11,7 @@ import { ISearchBoxProps, ISearchBoxStyleProps, ISearchBoxStyles } from './Searc | |||
|
|||
export function getStyles(props: ISearchBoxStyleProps): ISearchBoxStyles { | |||
const { theme, underlined, disabled, hasFocus, className, hasInput } = props; | |||
const { palette, fonts } = theme; | |||
const { palette, fonts, semanticColors } = theme; |
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.
shouldn't you get a compiler warning for adding semanticColors here but not use it?
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.
true! I just added the slot for inputPlaceholderText.
@@ -142,6 +142,16 @@ export function getStyles(props: ISearchBoxStyleProps): ISearchBoxStyles { | |||
selectors: { | |||
'::-ms-clear': { | |||
display: 'none' | |||
}, | |||
'::placeholder': { | |||
color: palette.neutralSecondary, |
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.
we should add an inputPlaceholderText slot
':-ms-input-placeholder': { | ||
color: palette.neutralSecondary | ||
}, | ||
'::-ms-input-placeholder': { |
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.
are both syntaxes necessary?
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.
Apparently browser versioning and placeholder selectors get hairy, but it looks like using just the two will work on at at least all my browser versions.
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.
Other than the duplicate syntax :ms and ::ms looks good to me
* Searchbox and textfield neutral secondary placeholder text * Using the palette * Added change file * New slot for inputPlaceholderText * Semantic slots in styles * style change file * Minor bump * Searchbox snapshot update
Pull request checklist
$ npm run change
Description of changes
Allows us to be consistent across browsers
Focus areas to test
(optional)