Skip to content
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

Cannot specify className of input on DateInput #404

Closed
mattdell opened this issue Jun 22, 2016 · 6 comments
Closed

Cannot specify className of input on DateInput #404

mattdell opened this issue Jun 22, 2016 · 6 comments

Comments

@mattdell
Copy link

I'm trying to include the DateTimePicker, which uses DateInput, into my existing solution that uses react-bootstrap. I want to have consistent styling on my inputs, which relies on my input having a class .form-control.

I don't see any means of including a class on the input via props. Can I suggest that we add this?

@mattdell
Copy link
Author

My proposal is something like...

DateInput.jsx

render(){
var value = this.state.textValue

return (
  <input
    {...this.props}
    type='text'
    className={cx({'rw-input': true, [this.props.inputClassName]: true })}
    value={value}
    aria-disabled={this.props.disabled}
    aria-readonly={this.props.readOnly}
    disabled={this.props.disabled}
    readOnly={this.props.readOnly}
    onChange={this._change}
    onBlur={chain(this.props.blur, this._blur, this)} />
)
},

I haven't used classnames before so not sure if that's the best syntax to use.

@jquense
Copy link
Owner

jquense commented Jun 22, 2016

You probably shouldn't be adding the form-control class to the inner input that will not do what you want at all. In general you should be trying to add classes to the inner components they are owned by the widget, and not really "public" if you do need to style stuff then i'd suggest adding the class the widget and using a css-selector

@mattdell
Copy link
Author

That is definitely what I want to do. Bootstrap targets the <input /> itself and adding .form-control to my input gives me the result that I want.

See their BaseInput: https://github.com/react-bootstrap/react-bootstrap/blob/master/src/InputBase.js

const className = this.isCheckboxOrRadio() || this.isFile() ? '' : 'form-control';
return <input {...this.props} className={classNames(this.props.className, className)} ref="input" key="input" />;

All inputs get .form-control unless it's a checkbox, radio, or a file input.

As the styles are vendor styles I can't add a css style to target input of a child without copying the styles from bootstrap and adding them into my custom css.

@jquense
Copy link
Owner

jquense commented Jun 22, 2016

As a maintainer of react-bootstrap I am aware of how it works ;)

Adding form-control to the input will style the inner input like its not part of a larger component. The "Input" here is the outer div. Also they are already styled like form-inputs what would be the advantage to adding it.

@mattdell
Copy link
Author

I stand corrected. :)

I see now I missed a crucial step where I need to import the styles for this repo as well. Embarassing.

@blackdynamo
Copy link

Sorry to bring this back up, as it's 5 years later. I am trying to format the date select so that it's a "floating label" which I have a lot of hackfu styles to make it happen. I am not sure if there is a simpler way to make the DatePicker a floating label component.

How this relates to the issue above also comes to play when I want to use form validation and make the date required. The required can be passed through using the inputProps but because the isn't a .form-control, it doesn't get the styles from validation. See attached image for an example. The date picker on the left was manually adding the .form-control class using the browser. You can see it now has the exclamation mark, because it's required but empty. The one on the left does not because it does not have the .form-control class.

image

This PR looks like it gave a way of passing the className down: #1004
This PR got merged and looks similar to the other PR: #1002

Any thoughts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants