React-bootstrap: Allow custom bsStyle

[janmarek]

Now you can't add classes like btn-custom or alert-custom to your stylesheet and use them with react-bootstrap components. bsStyle property should be validated less strictly.

[trentgrover-wf]

if you validate less strictly, you open yourself up to typo like problems (essentially you can't validate anything reliably at all). isn't it sufficient that you can provide btn-custom and such as optional css classes directly via className?

[janmarek]

Unfortunately no, there would be className "btn btn-default btn-custom" instead of "btn btn-custom".

[trentgrover-wf]

you can eliminate that by specifying bsStyle of empty string bsStyle=""

[janmarek]

Ok, thanks.

<a className="btn btn-custom">click</a>
<Button bsStyle="" className="btn-custom">click</Button>

But it's probably nicer not to use react-bootstrap in this situation...

[ivan-kleshnin]

@trentgrover-wf you can't rely on bootstrap draft semantics and call it "rules".
It was built for user extension. Bootstrap info, warning, etc. are there for demo purposes. End users may and should create their own sets of statuses etc. The current way of handling this is very uncomfortable, I 100% agree here with @janmarek. You don't need to validate class names against predefined sets.

[mtscout6]

I can see the value in doing some kind of validation against a known set of styles you support, yet I can also see the value in allowing for custom styles. I think you can add your own custom styles and still keep the existing validation by doing:

var constants = require('react-boostrap/constants');
constants.STYLES.custom = 'custom';

The name constants is deceiving here since nothing has been done to ensure that those values have been modified. If you did this as one of your initial tasks prior to rendering anything, then you'd have the custom options you want with the validation to support it.

[mathieumg]

Until the day these are enforced to be constant, or the filename changes.

[mtscout6]

We can cross that bridge when that day comes :smile:. I'm not planning to change that anytime soon.

[ivan-kleshnin]

@mtscout6 thank your for this trick, it may help.

[mtscout6]

No problem!

[mtscout6]

@janmarek Does the above suggestion help? Do we need to keep this issue open for any reason?

[janmarek]

OK, I'm closing it.

[nemosupremo]

If you set bsStyle="", you get a ton of console errors where

Warning: Invalid prop `bsStyle` of value `` supplied to `Button`, expected one of ["default","primary","success","info","warning","danger","link","inline","tabs","pills"]. Check the render method of `DropdownButton`.

Unfortunately, if you aren't using CommonJS/AMD, then constants isn't exposed either. If bsStyle="" isn't a valid property bsStyle={false} or something like that should be enabled to prevent this validation. For us, we use a modified version of the bootstrap css (which I think is pretty common) and having designers provide a JS file listing out all the btn classes (or having developers hunt down all the btn classes in css) they used in a design seems like an unnecessary build step.

[ivan-kleshnin]

@nemothekid, agree. Everyone modifies bootstrap for production. Nearly everyone adds custom class names for BS. I still hold my alredy expressed opionion that react-bootstrap devs just created a problem out of thin air with this. I don't care about class name validation at all (the cost of error is very low).

[mtscout6]

@nemothekid I'd take a pull request to fix the problem with the constants not being available when using requirejs.

[trevorr]

@mtscout6's workaround for webpack (missing t typo fixed and lib added):

var constants = require('react-bootstrap/lib/constants');
constants.STYLES.custom = 'custom';

It's not a perfect workaround because it has to be included before anything else includes react-bootstrap, since BootstrapMixin copies the keys when loaded.

[trevorr]

Did v0.20.0 break the workaround of modifying constants? It's not working for me anymore.

[mtscout6]

Thanks @trevorr I didn't realize that I missed that when we transitioned to using ES6 source. I just published a fix in version 0.20.1.

[AlexKVal]

Maybe it would be appropriate to implement some kind of configuration option for disabling
all style checking at all for those who needs it ?
Maybe as temporary solution ?

[mtscout6]

I don't think that's entirely necessary. All that happens with prop validation is console warnings. No errors, and #496 really did provide a nice means to include supported values by downstream users.

[AlexKVal]

It seems that hardcoded "bottlenecks" exists only in three components now.
<Input />

propTypes: {
  bsStyle: React.PropTypes.oneOf(['success', 'warning', 'error'])
...

and
<FormGroup /> bsStyle: React.PropTypes.oneOf(['success', 'warning', 'error']),
<ListGroupItem /> bsStyle: React.PropTypes.oneOf(['danger', 'info', 'success', 'warning'])

Is there any value to implement something like #496 for these three components ?
I don't have enough expertise in react-bootstrap for this to answer.

[mtscout6]

What's difficult about this is that bootstrap does not have the same set of styles for each component. For example the Button css styles support more types than Input of type text. It would be worth taking an inventory of each of the sets that bootstrap uses and make a decision with some better knowledge. What I think has happened is that we have been starting to see that disparity, and haven't figured out a decent solution for it.

[mtscout6]

The other problem is that some folks are still able to write their own css which may add more available styles. So, for those components these users want to allow those added styles without console warnings. Having the global pool of options through the old constants and now styleMaps though is sub-optimal since it would not catch usage of styles that may not exist for a particular component. This needs more thought....

[chellberg]

+1 @nemothekid, would be great if bsStyle={false} suppressed the warnings. I'm trying to conditionally apply bsStyle and there doesn't appear to be a falsey option that doesn't generate warnings.

(thanks for the great library, guys)

[bbtfr]

Seems we don't have react-bootstrap/lib/constants anymore?
+1 for bsStyle={false}
By the way, I found that it's possible to create a custom bsStyle for Button by changing styleMaps, but the bsStyle for Nav is hard-coded.

[taion]

This got fully replaced now. We're not dropping validation anyway.

[tonyspiro]

Running into this also. Please update.

[tonyspiro]

Ok, looks like bsStyle={ null } suppresses the errors.

[Dragonqos]

tonyspiro - thanks, bsStyle={null} works great

[bbtfr]

@tonyspiro :+1: thanks!

[thinkshihang]

@tonyspiro Thanks. bsStyle={null} +1