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

Difference between docs and implementation of Remaining Props in TextField #26

Closed
carlosforero opened this issue Apr 9, 2016 · 4 comments · Fixed by #31
Closed

Difference between docs and implementation of Remaining Props in TextField #26

carlosforero opened this issue Apr 9, 2016 · 4 comments · Fixed by #31
Labels

Comments

@carlosforero
Copy link
Contributor

The documentation of TextField says:

  • Any remaining props: Any other props (such as style, aria-tags, etc) will be applied to the container component.

However the TextField implementation applies to the Input instead of field container:

lib/TextFields/TextFields.js:194

      var textFieldProps = _extends({}, props, {
        value: value,
        className: (0, _classnames2.default)('md-text-field', className, {
          active: active,
          'floating-label': useFloatingLabel,
          'single-line': !useFloatingLabel && !multiline,
          'multi-line': multiline,
          'full-width': fullWidth
        }),
        onFocus: this.handleFocus,
        onBlur: this.handleBlur,
        onChange: this.handleChange
      });

I assume the right is in the container as the documentation says.

Other issue, the documentation says that style is a remaining property, but in the code style is filtered from the remaining properties:

lib/TextFields/TextFields.js:153

var props = _objectWithoutProperties(_props, ['className', 'containerClassName', 'label', 'placeholder', 'maxLength', 'helpText', 'errorText', 'floatingLabel', 'icon', 'rightIcon', 'lineDirection', 'rows', 'maxRows', 'style', 'required', 'helpOnFocus', 'fullWidth']);

Should style be applied to the input or should be a remaining property and applied to the container?

@mlaursen
Copy link
Owner

I believe the documentation should specify that the props are added onto the input component instead of the field container so that any event handlers can be passed down correctly. That should be a documentation update.

After looking through the code, I believe that the style should actually be applied to the container instead of the inout/textarea tag since it would be much more likely to want inline styling on the container.

@mlaursen mlaursen added the bug label Apr 10, 2016
@mlaursen mlaursen added this to the v0.1.0 Release milestone Apr 10, 2016
@carlosforero
Copy link
Contributor Author

Currently there are className which applies to the Input and containerClassName which applies to the container.

I think should be similar with style. Making explicit the style property for the Input and adding a containerStyle property. What do you think?

@mlaursen
Copy link
Owner

That sounds good to me. Will be a few lines shorter to implement :)

@carlosforero
Copy link
Contributor Author

Good, I can prepare a PR tomorrow (sunday) night.

This was referenced Apr 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants