[TextField] add floatingLabelFocusStyle prop #4043

echenley · 2016-04-19T21:54:31Z

adds floatingLabelFocusStyle prop to <TextField>
fixes More advanced component customization #2262

PR has tests / docs demo, and is linted.
Commit and PR titles begin with [ComponentName], and are in imperative form: "[Component] Fix leaky abstraction".
Description explains the issue / use-case resolved, and auto-closes the related issue(s) (http://tr.im/vFqem).

nathanmarks · 2016-04-19T23:03:28Z

src/TextField/TextFieldLabel.spec.js

+
+describe('<TextFieldLabel>', () => {
+  it('uses focus styles', () => {
+    const component = shallow(


Can you use wrapper instead of component?

echenley · 2016-04-20T20:33:32Z

Updates made. Let me know if there's anything else!

oliviertassinari · 2016-04-22T19:41:31Z

@echenley That looks good to me. Could you squash down and rebase? Thanks.

echenley · 2016-04-24T20:55:07Z

@oliviertassinari Should be good to go. Thank you!

oliviertassinari · 2016-04-24T21:01:40Z

src/TextField/TextField.js

+     * The style object to use to override floating label styles when focused.
+     */
+    floatingLabelFocusStyle: PropTypes.object,
+


Could you remove this line cf. #4073 😁?

oliviertassinari · 2016-04-24T21:20:33Z

@nathanmarks Feel free to merge if you are happy with this PR.

nathanmarks · 2016-04-24T21:28:05Z

src/TextField/TextFieldLabel.js

+    pointerEvents: 'none',
+  }, props.shrinkStyle) : null;
+
+  return Object.assign({


@oliviertassinari should we enforce the styles.root convention here?

@nathanmarks I would say yes 👍.

echenley · 2016-04-24T21:38:53Z

Updated:

sort TextFieldLabel props alphabetically
use styles.root pattern

nathanmarks · 2016-04-24T21:39:49Z

src/TextField/TextFieldLabel.js

+    cursor: props.disabled ? 'default' : 'text',
+    transform: 'scale(1) translate3d(0, 0, 0)',
+    transformOrigin: 'left top',
+    pointerEvents: 'auto',


Can we remove pointerEvents: 'none', out of the TextField.js~getStyles().floatingLabel object? seems like the default is already enforced here. (here being in the getStyles() function as a whole, not this line)

(do correct me if I'm wrong though!)

You're right, it was redundant. I wonder if it might be better to just move the defaultStyles in TextFieldLabel to styles.floatingLabel in TextField instead of the other way? That would remove one object merge from the mix. I'll make a commit to show what I mean.

See f8a4a63.

I wonder if it might be better to just move the defaultStyles in TextFieldLabel to styles.floatingLabel in TextField instead of the other way?

The idea is to make each component independent. I think that we shouldn't move the style to the parent component.

nathanmarks · 2016-04-24T21:55:37Z

@oliviertassinari TextFieldUnderline accesses muiTheme itself to set defaults for the component: https://github.com/callemall/material-ui/blob/master/src/TextField/TextFieldUnderline.js#L64 Whereas TextField sets the defaults for TextFieldLabel. I think these should be consistent with each other.

nathanmarks · 2016-04-24T21:58:41Z

The TextFieldLabel may as well access the theme variables itself for the colour in order to set those 2 defaults currently passed in by TextField.

oliviertassinari · 2016-04-24T22:01:03Z

@nathanmarks I agree 👍. Let's remove the color: hintColor, from the TextField.

oliviertassinari · 2016-04-24T22:02:11Z

@nathanmarks Humm, I'm not sure though. There is like 10 lines of logic to update the color after behing set.

nathanmarks · 2016-04-24T22:04:39Z

@oliviertassinari bleh, that's annoying.

oliviertassinari · 2016-04-24T22:05:14Z

@echenley Could you remove the second commit? Let's try to keep separate our concerns.

nathanmarks · 2016-04-24T22:05:45Z

I guess it just feels strange because it is an escape hatch by design through to the component during the focus state of the input the parent contains (these types of props).

echenley · 2016-04-24T22:11:54Z

Yeah, the only alternative seems to be passing TextField state (or at least isFocused
hasValue and errorText) into TextFieldLabel and doing the logic there.

nathanmarks · 2016-04-24T22:12:54Z

@echenley yeah, lesser of evils 😄

This is one of those annoying cases too where even JS non-inline styles won't make it cleaner either because the label doesn't actually get focussed when the input does, ah well!

nathanmarks · 2016-04-24T22:20:38Z

@oliviertassinari

I've noticed a problem with the label color while checking out this PR in the docs -- not just on this PR but also on HEAD and 0.14.4 too.... should we fix this here?

MUI Docs

Spec

echenley · 2016-04-24T22:31:37Z

@nathanmarks Strange, I don't see this when I run the docs locally or here: http://www.material-ui.com/v0.15.0-beta.2/#/components/text-field

Edit: oh, I see it now. If it's focused and has text, it reverts to the default style. I'll add a fix.

echenley · 2016-04-24T22:45:23Z

src/TextField/TextField.js

  if (state.hasValue) {
    styles.floatingLabel.color = fade(props.disabled ? disabledTextColor : floatingLabelColor, 0.5);
  }

+  if (state.isFocused) {
+    styles.floatingLabel.color = focusColor;
+  }


@nathanmarks Switching the order here fixes the focus color issue. Unless I'm missing why hasValue was overriding it.

oliviertassinari · 2016-04-25T19:02:29Z

@echenley Nice PR, thanks👍.

nathanmarks · 2016-04-25T20:26:03Z

@echenley Thanks for sorting that bug!!!

Sabst · 2016-04-30T15:55:02Z

Looks like after 0.15.0-beta.2 (included) this will allow for a dense text field.
Is this correct?
I read the different issues and the associated doc and still don't see how this should be used.
Let's say I need a text with a dense floating label (60dp high instead of the default 72dp).
Can you elaborate a bit more on how that works or provide a pointer to an example or doc?

echenley changed the title ~~[TextField] adds floatingLabelFocusStyle prop~~ [TextField] add floatingLabelFocusStyle prop Apr 19, 2016

nathanmarks reviewed Apr 19, 2016
View reviewed changes

oliviertassinari added the PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI label Apr 20, 2016

nathanmarks mentioned this pull request Apr 22, 2016

[Core] There should be a more sophisticated styling solution #4066

Closed

oliviertassinari added PR: Review Accepted PR: out-of-date The pull request has merge conflicts and can't be merged and removed PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI labels Apr 22, 2016

echenley force-pushed the feature/floatingLabelFocusStyle branch 2 times, most recently from f28bdf1 to 8904c37 Compare April 24, 2016 20:45

oliviertassinari reviewed Apr 24, 2016
View reviewed changes

oliviertassinari removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Apr 24, 2016

echenley force-pushed the feature/floatingLabelFocusStyle branch from 8904c37 to 027c96d Compare April 24, 2016 21:04

nathanmarks reviewed Apr 24, 2016
View reviewed changes

echenley force-pushed the feature/floatingLabelFocusStyle branch from 027c96d to bd92715 Compare April 24, 2016 21:36

nathanmarks reviewed Apr 24, 2016
View reviewed changes

echenley force-pushed the feature/floatingLabelFocusStyle branch from bd92715 to b800fe1 Compare April 24, 2016 21:45

echenley force-pushed the feature/floatingLabelFocusStyle branch from f8a4a63 to b800fe1 Compare April 24, 2016 22:06

[TextField] add floatingLabelFocusStyles prop

f82b3ac

echenley force-pushed the feature/floatingLabelFocusStyle branch from b800fe1 to f82b3ac Compare April 24, 2016 22:42

echenley reviewed Apr 24, 2016
View reviewed changes

oliviertassinari merged commit c824ad8 into mui:master Apr 25, 2016

zannager added the component: text field This is the name of the generic UI component, not the React module! label Mar 14, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[TextField] add floatingLabelFocusStyle prop #4043

[TextField] add floatingLabelFocusStyle prop #4043

echenley commented Apr 19, 2016 •

edited by oliviertassinari

Loading

nathanmarks Apr 19, 2016

echenley commented Apr 20, 2016 •

edited

Loading

oliviertassinari commented Apr 22, 2016

echenley commented Apr 24, 2016

oliviertassinari Apr 24, 2016

oliviertassinari commented Apr 24, 2016

nathanmarks Apr 24, 2016

oliviertassinari Apr 24, 2016

echenley commented Apr 24, 2016

nathanmarks Apr 24, 2016 •

edited

Loading

echenley Apr 24, 2016

nathanmarks Apr 24, 2016

echenley Apr 24, 2016 •

edited

Loading

echenley Apr 24, 2016

oliviertassinari Apr 24, 2016

nathanmarks commented Apr 24, 2016

nathanmarks commented Apr 24, 2016

oliviertassinari commented Apr 24, 2016

oliviertassinari commented Apr 24, 2016 •

edited

Loading

nathanmarks commented Apr 24, 2016

oliviertassinari commented Apr 24, 2016

nathanmarks commented Apr 24, 2016 •

edited

Loading

echenley commented Apr 24, 2016

nathanmarks commented Apr 24, 2016

nathanmarks commented Apr 24, 2016 •

edited

Loading

echenley commented Apr 24, 2016 •

edited

Loading

echenley Apr 24, 2016

oliviertassinari commented Apr 25, 2016

nathanmarks commented Apr 25, 2016

Sabst commented Apr 30, 2016

[TextField] add floatingLabelFocusStyle prop #4043

[TextField] add floatingLabelFocusStyle prop #4043

Conversation

echenley commented Apr 19, 2016 • edited by oliviertassinari Loading

Choose a reason for hiding this comment

echenley commented Apr 20, 2016 • edited Loading

oliviertassinari commented Apr 22, 2016

echenley commented Apr 24, 2016

Choose a reason for hiding this comment

oliviertassinari commented Apr 24, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

echenley commented Apr 24, 2016

nathanmarks Apr 24, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

echenley Apr 24, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nathanmarks commented Apr 24, 2016

nathanmarks commented Apr 24, 2016

oliviertassinari commented Apr 24, 2016

oliviertassinari commented Apr 24, 2016 • edited Loading

nathanmarks commented Apr 24, 2016

oliviertassinari commented Apr 24, 2016

nathanmarks commented Apr 24, 2016 • edited Loading

echenley commented Apr 24, 2016

nathanmarks commented Apr 24, 2016

nathanmarks commented Apr 24, 2016 • edited Loading

MUI Docs

Spec

echenley commented Apr 24, 2016 • edited Loading

Choose a reason for hiding this comment

oliviertassinari commented Apr 25, 2016

nathanmarks commented Apr 25, 2016

Sabst commented Apr 30, 2016

echenley commented Apr 19, 2016 •

edited by oliviertassinari

Loading

echenley commented Apr 20, 2016 •

edited

Loading

nathanmarks Apr 24, 2016 •

edited

Loading

echenley Apr 24, 2016 •

edited

Loading

oliviertassinari commented Apr 24, 2016 •

edited

Loading

nathanmarks commented Apr 24, 2016 •

edited

Loading

nathanmarks commented Apr 24, 2016 •

edited

Loading

echenley commented Apr 24, 2016 •

edited

Loading