-
-
Notifications
You must be signed in to change notification settings - Fork 32.4k
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
[TextField] add floatingLabelFocusStyle prop #4043
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,31 @@ | ||
import React, {PropTypes} from 'react'; | ||
import transitions from '../styles/transitions'; | ||
|
||
function getStyles(props) { | ||
const defaultStyles = { | ||
position: 'absolute', | ||
lineHeight: '22px', | ||
top: 38, | ||
transition: transitions.easeOut(), | ||
zIndex: 1, // Needed to display label above Chrome's autocomplete field background | ||
cursor: props.disabled ? 'default' : 'text', | ||
transform: 'scale(1) translate3d(0, 0, 0)', | ||
transformOrigin: 'left top', | ||
pointerEvents: 'auto', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we remove There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (do correct me if I'm wrong though!) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See f8a4a63. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The idea is to make each component independent. I think that we shouldn't move the style to the parent component. |
||
userSelect: 'none', | ||
}; | ||
|
||
const shrinkStyles = props.shrink ? Object.assign({ | ||
transform: 'perspective(1px) scale(0.75) translate3d(0, -28px, 0)', | ||
pointerEvents: 'none', | ||
}, props.shrinkStyle) : null; | ||
|
||
return { | ||
root: Object.assign(defaultStyles, props.style, shrinkStyles), | ||
}; | ||
} | ||
|
||
const propTypes = { | ||
/** | ||
* @ignore | ||
* The material-ui theme applied to this component. | ||
*/ | ||
muiTheme: PropTypes.object.isRequired, | ||
/** | ||
* The css class name of the root element. | ||
*/ | ||
|
@@ -19,18 +38,27 @@ const propTypes = { | |
* Disables the label if set to true. | ||
*/ | ||
disabled: PropTypes.bool, | ||
/** | ||
* True if the floating label should shrink. | ||
*/ | ||
shrink: PropTypes.bool, | ||
/** | ||
* The id of the target element that this label should refer to. | ||
*/ | ||
htmlFor: PropTypes.string, | ||
/** | ||
* @ignore | ||
* The material-ui theme applied to this component. | ||
*/ | ||
muiTheme: PropTypes.object.isRequired, | ||
/** | ||
* Callback function for when the label is selected via a touch tap. | ||
*/ | ||
onTouchTap: PropTypes.func, | ||
/** | ||
* True if the floating label should shrink. | ||
*/ | ||
shrink: PropTypes.bool, | ||
/** | ||
* Override the inline-styles of the root element when focused. | ||
*/ | ||
shrinkStyle: PropTypes.object, | ||
/** | ||
* Override the inline-styles of the root element. | ||
*/ | ||
|
@@ -47,36 +75,17 @@ const TextFieldLabel = (props) => { | |
muiTheme, | ||
className, | ||
children, | ||
disabled, | ||
shrink, | ||
htmlFor, | ||
style, | ||
onTouchTap, | ||
} = props; | ||
|
||
const styles = { | ||
root: { | ||
position: 'absolute', | ||
lineHeight: '22px', | ||
top: 38, | ||
transition: transitions.easeOut(), | ||
zIndex: 1, // Needed to display label above Chrome's autocomplete field background | ||
cursor: disabled ? 'default' : 'text', | ||
transform: shrink ? | ||
'perspective(1px) scale(0.75) translate3d(0, -28px, 0)' : | ||
'scale(1) translate3d(0, 0, 0)', | ||
transformOrigin: 'left top', | ||
pointerEvents: shrink ? 'none' : 'auto', | ||
userSelect: 'none', | ||
}, | ||
}; | ||
|
||
const {prepareStyles} = muiTheme; | ||
const styles = getStyles(props); | ||
|
||
return ( | ||
<label | ||
className={className} | ||
style={prepareStyles(Object.assign({}, styles.root, style))} | ||
style={prepareStyles(styles.root)} | ||
htmlFor={htmlFor} | ||
onTouchTap={onTouchTap} | ||
> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
/* eslint-env mocha */ | ||
import React from 'react'; | ||
import {shallow} from 'enzyme'; | ||
import {expect} from 'chai'; | ||
import TextFieldLabel from './TextFieldLabel'; | ||
import getMuiTheme from '../styles/getMuiTheme'; | ||
|
||
describe('<TextFieldLabel>', () => { | ||
it('uses focus styles', () => { | ||
const wrapper = shallow( | ||
<TextFieldLabel | ||
muiTheme={getMuiTheme()} | ||
shrink={false} | ||
style={{color: 'regularcolor'}} | ||
shrinkStyle={{color: 'focuscolor'}} | ||
/> | ||
); | ||
|
||
expect(wrapper.type()).to.equal('label'); | ||
expect(wrapper.prop('style').color).to.equal('regularcolor'); | ||
|
||
wrapper.setProps({shrink: true}); | ||
expect(wrapper.prop('style').color).to.equal('focuscolor'); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nathanmarks Switching the order here fixes the focus color issue. Unless I'm missing why hasValue was overriding it.