[TextField] Add support for "secondary" color #17891
Conversation
|
@material-ui/core: parsed: +0.16% , gzip: +0.09% Details of bundle changes.Comparing: 258e847...29ba262
|
There was a problem hiding this comment.
Should the color propagate through the form context?
do we want to support "inherit"?
I have explored it a bit in the past, I didn't find any good lead. Did you find a possible solution?
which CSS rule name should we use? I've used both secondaryColor and underlineSecondary...
colorSecondary?
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 48d6cab:
|
|
I didn't pay attention to the context, I'll look into that. Regarding inherit, I was just asking because I saw it on other components. Indeed, it's probably tricky to support it here. I'm fine with I'll update this later today 🙂 |
|
@oliviertassinari I've made the requested adjustments. I didn't use the form context inside |
ValentinH
force-pushed
the
input-secondary-color
branch
3 times, most recently
from
ed5f9b3 to
068beea
Compare
oliviertassinari
added
the
component: text field
oliviertassinari
changed the title
oliviertassinari
force-pushed
the
input-secondary-color
branch
2 times, most recently
from
2e44eb6 to
24721c0
Compare
oliviertassinari
changed the title
oliviertassinari
force-pushed
the
input-secondary-color
branch
2 times, most recently
from
b0bef5f to
c8cd861
Compare
| }); | ||
|
|
||
| return ( | ||
| <Component | ||
| className={clsx( | ||
| classes.root, | ||
| classes[`color${capitalize(fcs.color || 'primary')}`], |
There was a problem hiding this comment.
Where is the colorPrimary class defined?
There was a problem hiding this comment.
It's not, it doesn't stop somebody to use .MuiFormLabel-colorPrimary (in the future)
|
I've checked your changes and it looks good to me too. 🎉 |
|
Great, let's wait till Monday to collect more feedback. |
As discussed in mui#17006 Open questions: - do we want to support "inherit"? - which CSS rule name should we use? I've used both `secondaryColor` and `underlineSecondary`...
oliviertassinari
force-pushed
the
input-secondary-color
branch
from
c8cd861 to
29ba262
Compare
|
@ValentinH Thank you |
|
@oliviertassinari why did you remove the demos? |
|
Also I just noticed that you changed the default color for the Input, FilledInput and Label from light/dark to main primary. Isn't this a breaking change? Or it's a bug fix? |
|
@ValentinH I have removed the demo because I think that few people will want to change the color from primary to secondary. I have changed the color to simplify the logic. |
|
Well, maybe there are more need for this color prop documentation demo than I thought: #11644 |
|
Getting lots of errors from libraries like |
|
It's not supported |
|
@jadbox are they typescript error or runtime ones? |
|
@ValentinH They are runtime errors. Here's the GH issue within |
|
Yes these are warnings only. Indeed, it's a side effect of a prop that used to be passed but not used. |
|
I seem to have the same problem as @jadbox regarding the When I use the input component:
The following warnings are generated in the console:
Is there any workaround to fix this? I'm using Material UI 4.6.1 |
|
Having the same issue. |
|
Really annoying. Can't get rid of it |
Closes #17006
Closes #17891
Open questions:
do we want to support "inherit"?
which CSS rule name should we use? I've used both
secondaryColorandunderlineSecondary...I have followed (at least) the PR section of the contributing guide.