formsy / formsy-material-ui

A Formsy compatibility wrapper for Material-UI form components
MIT License
571 stars 149 forks source link

valid FormsyText will overwrite underlineFocusStyle border-color style with default color #194

Open cryptokoala4 opened 7 years ago

cryptokoala4 commented 7 years ago

It seems that the style set inside underlineFocusStyle will be overwritten with default color when the FormsyText validation is passed. Very annoying bug since the default is bright blue.

Any workaround, or will I have to disable underline in the meantime?

Using : "react": "15.4.2", "formsy-material-ui": "^0.5.5", "formsy-react": "^0.18.0",

cryptokoala4 commented 7 years ago

To try and be helpful, adding this code in line 152 of app/Main.js inside the example code will let you see the bug:

underlineFocusStyle={{borderColor:'green'}}

You will see that when you click on the Name field, the color shows green (as underlineFocusStyle intended: http://www.material-ui.com/#/components/text-field). But when you enter text into this field, it turns blue, because formsy somehow defaults the color back to material-ui's default blue color upon validation, overwriting the style you defined inside underlineFocusStyle.

Any quick fix for this while it's being looked at?

ryanblakeley commented 7 years ago

Just my initial thoughts because I don't have time to dig into this right now:
We recently accepted a change that added the underlineStyle and underlineFocusStyle props, see here. It probably has to do with this.validationColor() being checked for a value before deferring to underlineStyle. There might be a theme variable for material-ui that sets the validation color. So there may be 3 or 4 total color settings you have to account for: basic, focused, and valid, invalid; with the later two being set through your material-ui theme. Just speculating though.

As a side note--does anyone have an issue with allowing the underlineStyle and underlineFocusStyle props? Or should those be delegated to the theme? Seems like it could be a convenience/confusion tradeoff.

kdichev commented 7 years ago

Guys this needs attention, lots of project use the lib in production. Bugs been happening for a while.

for the time being this could be a simple hot-fix till the issue is resolved.

import MuiThemeProvider from 'material-ui/styles/MuiThemeProvider'; import getMuiTheme from 'material-ui/styles/getMuiTheme';

const muiTheme = getMuiTheme({}, { palette: { primary1Color: "YOUR_CHOICE_OF_COLOR", } });

and add it to the render

<MuiThemeProvider muiTheme={muiTheme}> ............ </MuiThemeProvider>

ryanblakeley commented 7 years ago

@kdichev your first sentence--there's no place for that here. This is an open source project, not a product that you paid for. Every bit of progress happens on a voluntary basis. If you're using this in production and a bug pops up that you don't think is being fixed quickly enough--dig into the library and offer some insight, a PR, or a question.

Aside from that, thanks for pointing out how to use the theme provider for those not familiar. I reckon there is a simple fix, but I have higher priorities this week. Maybe Saturday if this is still open, I'll jump in.

kdichev commented 7 years ago

Hey I totally did not have an intend to sound like that, demanding... I'm a young dev so I don't know the codex. I'll checkout the source and try to fix it :)

cryptokoala4 commented 7 years ago

Hey thanks for acknowledging the bug. I was worried I was the only one experiencing it since nobody else had raised the issue, as it seems to be quite damaging.

I don't have experience fixing libraries, but I'll follow this issue closely and hopefully learn how the eventual fix goes.