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

Ability to style Textfield elements using inline styles #58

Open
ivmarkov opened this issue Nov 22, 2017 · 4 comments
Open

Ability to style Textfield elements using inline styles #58

ivmarkov opened this issue Nov 22, 2017 · 4 comments
Assignees

Comments

@ivmarkov
Copy link

ivmarkov commented Nov 22, 2017

See #57.
For <Textfield> there is a similar (but not identical) issue. "...otherProps" are put on the <input> element, and not on the <div> (Field) element. While I see the rationale for this (very often you would like to push properties to the input element, as this is the one which interacts with the user), this precludes us from styling the Textfield element as a whole. For example, how am I supposed to make it occupy 100% width? Perhaps, I'm missing the elephant here...

@kradio3 kradio3 self-assigned this Nov 23, 2017
@ivmarkov
Copy link
Author

ivmarkov commented Nov 27, 2017

The problem is amplified when the Textfield element has a helptext, because then, besides the Field inner component, there is a div element wrapping the Field and the Helptext components which also needs styling.

The more I think about it, the more the issue seems to be related to styling only. In other words:

  • It is good that in general, the props go to the "most important" element. For Textfield, that would always be the input element, as it is now. For other components like Dialog, we have to think. Note though that other frameworks like Material-UI (see below), besides assigning the props to the most important inner element, also define optional inputProps, helpProps, etc., but I think that's completely orthogonal to the styling issue, and may or may not be implemented later
  • For styling however, it is important that there are facilities, with which each inner element in the e.g. composite Textfield control can be styled

Perhaps it might help if we learn how other toolkits are doing it. Here's the rule in Material-UI:

  • Rule 1 - styling with inline styles (note: not sure to what extent they'll keep this rule in the upcoming v1!): They always put the style property on the root element, and then, for each inner element, they expose a xxxStyle property
  • Rule 2 - styling with CSS classes (very convenient in conjunction with a CSS-in-JS framework like JSS, emotion etc. which allows the CSS styles to remain local to the control being styled): Material-UI always assigns the user-supplied "className" property to the root element of the composite control. This allows then the user to supply - for className- a CSS rule which - via CSS selectors - might style any of the sub-elements in the DOM tree of the composite control.

@ivmarkov
Copy link
Author

ivmarkov commented Nov 27, 2017

OK. So in the upcoming v1, Material UI seems to have given up completely on "Rule 1" from above. "xxxStyles" are nowhere to be seen. "Rule 2" seems to be 100% enforced though. The only thing they allow, in their Textfield Props is:

  • An optional user-supplied "className" string prop for the root component in the Textfield component three. They obviously do have a "style" property, but it seems to be always assigned to the root component, and they don't deal with it in any explicit way.
  • Optional user-supplied "xxxClassName" string properties for the major sub-components (Input, Helptext, etc.) which are appended to their own internal styling classNames (they seem to now do styling completely with JSS embedded rules)

Would that work for us? E.g. follow a simple pattern where the user can supply a className string prop which is always assigned to the root element of the component, as well as one or more "xxxClassName" properties that - if supplied - will be assigned to the major sub-elements in the component's element tree? This seems like a simple change, which has the added benefit that it can be used with a CSS-in-JS or with regular CSS rules.

@ivmarkov
Copy link
Author

Any feedback? Ability to style components is crucial if the library is to gain traction in the community...

@kradio3
Copy link
Owner

kradio3 commented Nov 28, 2017

Thank you for analyze, sure it make sense. I'll extend components by xxxClassName props.

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

2 participants