-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
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] Make it meet guidelines #6566
Changes from 25 commits
7f966d0
10a5c0f
6e8ad06
b5fb82a
6cece85
7b7fa13
4716659
6f83e5b
186a251
7abcec9
d02154c
dffc976
890217f
0b15230
f27ea99
b11a2e8
7a3d1f4
710e605
ff1abe5
0061db1
6af7c88
0565e94
c3bde34
c740da2
760eb25
4e16f7c
e7e7f47
38d4043
2f70c6c
1457b96
5fe667a
5d6970d
612b02c
ae3d010
3cd4b99
9b40eee
428d870
489de60
a8a9a6c
3043877
f30dbea
0419922
c37fe6e
30be343
60ae4d7
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,30 +1,26 @@ | ||
# TextFieldLabel | ||
FormHelperText | ||
========= | ||
|
||
|
||
|
||
## Props | ||
| Name | Type | Default | Description | | ||
|:-----|:-----|:--------|:------------| | ||
| children | node | | The content of the component. | | ||
| classes | object | | Useful to extend the style applied to components. | | ||
| disableAnimation | bool | false | If `true`, the transition animation is disabled. | | ||
| error | bool | | If `true`, the label is displayed in an error state. | | ||
| focused | bool | | If `true`, the input of this label is focused. | | ||
| required | bool | | If `true`, the label will indicate that the input is required. | | ||
| shrink | bool | false | If `true`, the label is shrunk. | | ||
| className | string | | The CSS class name of the root element. | | ||
| error | bool | | Whether the helper text should be displayed in an error state. | | ||
|
||
Any other properties supplied will be spread to the root element. | ||
## Classes | ||
|
||
You can overrides all the class names injected by Material-UI thanks to the `classes` property. | ||
This property accepts the following keys: | ||
- `root` | ||
- `shrink` | ||
- `animated` | ||
- `error` | ||
|
||
Have a look at [overriding with class names](/customization/overrides#overriding-with-class-names) | ||
section for more detail. | ||
|
||
If using the `overrides` key of the theme as documented | ||
[here](/customization/themes#customizing-all-instances-of-a-component-type), | ||
you need to use the following style sheet name: `MuiTextFieldLabel`. | ||
you need to use the following style sheet name: `MuiFormHelperText`. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
--- | ||
components: Input, TextField, TextFieldLabel | ||
components: Input, TextField, FormHelperText | ||
--- | ||
|
||
# Text Fields | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,11 @@ | ||
// @flow weak | ||
|
||
import React, { Component } from 'react'; | ||
import React, { Children, Component } from 'react'; | ||
import PropTypes from 'prop-types'; | ||
import classNames from 'classnames'; | ||
import { createStyleSheet } from 'jss-theme-reactor'; | ||
import withStyles from '../styles/withStyles'; | ||
import { isDirty } from '../Input/Input'; | ||
|
||
export const styleSheet = createStyleSheet('MuiFormControl', { | ||
root: { | ||
|
@@ -49,6 +50,19 @@ class FormControl extends Component { | |
}; | ||
} | ||
|
||
componentWillMount() { | ||
Children.forEach(this.props.children, child => { | ||
if ( | ||
child && | ||
child.type && | ||
(child.type.name === 'Input' || (child.type.Naked && child.type.Naked.name === 'Input')) && | ||
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. I'm not sure we want to rely on 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. @oliviertassinari Thought |
||
isDirty(child.props) | ||
) { | ||
this.setState({ dirty: true }); | ||
} | ||
}); | ||
} | ||
|
||
handleFocus = () => { | ||
if (this.props.onFocus) { | ||
this.props.onFocus(); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,69 @@ | ||
// @flow weak | ||
|
||
import React from 'react'; | ||
import PropTypes from 'prop-types'; | ||
import classNames from 'classnames'; | ||
import { createStyleSheet } from 'jss-theme-reactor'; | ||
import withStyles from '../styles/withStyles'; | ||
|
||
export const styleSheet = createStyleSheet('MuiFormHelperText', theme => ({ | ||
root: { | ||
color: theme.palette.input.helperText, | ||
fontSize: 12, | ||
lineHeight: 1.4, | ||
margin: 0, | ||
}, | ||
error: { | ||
color: theme.palette.error.A400, | ||
}, | ||
})); | ||
|
||
function FormHelperText(props, context) { | ||
const { children, classes, className: classNameProp, error: errorProp, ...other } = props; | ||
const { muiFormControl } = context; | ||
|
||
let error = errorProp; | ||
|
||
if (muiFormControl && typeof error === 'undefined') { | ||
error = muiFormControl.error; | ||
} | ||
|
||
const className = classNames( | ||
classes.root, | ||
{ | ||
[classes.error]: error, | ||
}, | ||
classNameProp, | ||
); | ||
|
||
return ( | ||
<p className={className} {...other}> | ||
{children} | ||
</p> | ||
); | ||
} | ||
|
||
FormHelperText.propTypes = { | ||
/** | ||
* The content of the component. | ||
*/ | ||
children: PropTypes.node, | ||
/** | ||
* Useful to extend the style applied to components. | ||
*/ | ||
classes: PropTypes.object.isRequired, | ||
/** | ||
* The CSS class name of the root element. | ||
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. We went with ignoring those from the documentation as quite idiomatic in the React world: /**
* @ignore
*/ |
||
*/ | ||
className: PropTypes.string, | ||
/** | ||
* Whether the helper text should be displayed in an error state. | ||
*/ | ||
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. 'If |
||
error: PropTypes.bool, | ||
}; | ||
|
||
FormHelperText.contextTypes = { | ||
muiFormControl: PropTypes.object, | ||
}; | ||
|
||
export default withStyles(styleSheet)(FormHelperText); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,63 @@ | ||
// @flow | ||
|
||
import React from 'react'; | ||
import { assert } from 'chai'; | ||
import { createShallow } from '../test-utils'; | ||
import FormHelperText, { styleSheet } from './FormHelperText'; | ||
|
||
describe('<FormHelperText />', () => { | ||
let shallow; | ||
let classes; | ||
|
||
before(() => { | ||
shallow = createShallow({ dive: true }); | ||
classes = shallow.context.styleManager.render(styleSheet); | ||
}); | ||
|
||
it('should render a <p />', () => { | ||
const wrapper = shallow(<FormHelperText className="woof" />); | ||
assert.strictEqual(wrapper.name(), 'p'); | ||
assert.strictEqual(wrapper.hasClass(classes.root), true, 'should have the root class'); | ||
assert.strictEqual(wrapper.hasClass('woof'), true, 'should have the user class'); | ||
}); | ||
|
||
describe('prop: error', () => { | ||
it('should show an error class', () => { | ||
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. 'should have an error class' / 'should set the error class'? |
||
const wrapper = shallow(<FormHelperText error />); | ||
assert.strictEqual(wrapper.hasClass(classes.error), true); | ||
}); | ||
}); | ||
|
||
describe('with muiFormControl context', () => { | ||
let wrapper; | ||
let muiFormControl; | ||
|
||
function setFormControlContext(muiFormControlContext) { | ||
muiFormControl = muiFormControlContext; | ||
wrapper.setContext({ ...wrapper.context(), muiFormControl }); | ||
} | ||
|
||
beforeEach(() => { | ||
wrapper = shallow(<FormHelperText>Foo</FormHelperText>); | ||
}); | ||
['error'].forEach(visualState => { | ||
describe(visualState, () => { | ||
beforeEach(() => { | ||
setFormControlContext({ [visualState]: true }); | ||
}); | ||
|
||
it(`should have the ${visualState} class`, () => { | ||
assert.strictEqual(wrapper.hasClass(classes[visualState]), true); | ||
}); | ||
|
||
it('should be overridden by props', () => { | ||
assert.strictEqual(wrapper.hasClass(classes[visualState]), true); | ||
wrapper.setProps({ [visualState]: false }); | ||
assert.strictEqual(wrapper.hasClass(classes[visualState]), false); | ||
wrapper.setProps({ [visualState]: true }); | ||
assert.strictEqual(wrapper.hasClass(classes[visualState]), true); | ||
}); | ||
}); | ||
}); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,18 +8,18 @@ import { createStyleSheet } from 'jss-theme-reactor'; | |
import withStyles from '../styles/withStyles'; | ||
|
||
export const styleSheet = createStyleSheet('MuiFormLabel', theme => { | ||
const focusColor = theme.palette.primary[500]; | ||
const focusColor = theme.palette.primary[theme.palette.type === 'light' ? 'A700' : 'A200']; | ||
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.
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. According to the latest specs it should be accent color. 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. Where do you see that? I have found the following:
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. 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. Oh, sounds good then 👍 |
||
return { | ||
root: { | ||
fontFamily: theme.typography.fontFamily, | ||
color: theme.palette.text.secondary, | ||
color: theme.palette.input.labelText, | ||
lineHeight: 1, | ||
}, | ||
focused: { | ||
color: focusColor, | ||
}, | ||
error: { | ||
color: theme.palette.error[500], | ||
color: theme.palette.error.A400, | ||
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. Same comment regarding |
||
}, | ||
}; | ||
}); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,7 +7,7 @@ import { createStyleSheet } from 'jss-theme-reactor'; | |
import withStyles from '../styles/withStyles'; | ||
import Textarea from './Textarea'; | ||
|
||
function isDirty(obj) { | ||
export function isDirty(obj) { | ||
return obj && obj.value && obj.value.length > 0; | ||
} | ||
|
||
|
@@ -19,12 +19,15 @@ export const styleSheet = createStyleSheet('MuiInput', theme => ({ | |
fontFamily: theme.typography.fontFamily, | ||
}, | ||
formControl: { | ||
marginTop: 10, | ||
marginBottom: 10, | ||
marginTop: 8, | ||
marginBottom: 8, | ||
'label + &': { | ||
marginTop: 32, | ||
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. Oh, why changing that? 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. What about |
||
}, | ||
}, | ||
inkbar: { | ||
'&:after': { | ||
backgroundColor: theme.palette.primary[500], | ||
backgroundColor: theme.palette.primary[theme.palette.type === 'light' ? 'A700' : 'A200'], | ||
left: 0, | ||
bottom: -2, | ||
// Doing the other way around crash on IE11 "''" https://github.com/cssinjs/jss/issues/242 | ||
|
@@ -45,21 +48,21 @@ export const styleSheet = createStyleSheet('MuiInput', theme => ({ | |
focused: {}, | ||
error: { | ||
'&:after': { | ||
backgroundColor: theme.palette.error[500], | ||
backgroundColor: theme.palette.error.A400, | ||
transform: 'scaleX(1)', // error is always underlined in red | ||
}, | ||
}, | ||
input: { | ||
font: 'inherit', | ||
padding: '6px 0', | ||
padding: '8px 0', | ||
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 can use |
||
border: 0, | ||
display: 'block', | ||
boxSizing: 'content-box', | ||
verticalAlign: 'middle', | ||
whiteSpace: 'normal', | ||
background: 'none', | ||
margin: 0, // Reset for Safari | ||
color: theme.palette.text.primary, | ||
color: theme.palette.input.inputText, | ||
width: '100%', | ||
'&:focus': { | ||
outline: 0, | ||
|
@@ -84,9 +87,19 @@ export const styleSheet = createStyleSheet('MuiInput', theme => ({ | |
color: theme.palette.text.disabled, | ||
}, | ||
underline: { | ||
borderBottom: `1px solid ${theme.palette.text.divider}`, | ||
borderBottom: `1px solid ${theme.palette.input.bottomLine}`, | ||
'&:hover:not($disabled)': { | ||
borderBottom: `2px solid ${theme.palette.text.primary}`, | ||
marginBottom: 7, | ||
transition: theme.transitions.create('border-color', { | ||
duration: theme.transitions.duration.shorter, | ||
easing: theme.transitions.easing.ease, | ||
}), | ||
}, | ||
'&$disabled': { | ||
borderBottomStyle: 'dotted', | ||
borderImage: `linear-gradient(to right, ${theme.palette.input.bottomLine} 33%, transparent 0%) | ||
100 0 / 0 0 1px / 0 0 0 3px repeat`, | ||
}, | ||
}, | ||
})); | ||
|
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.
That's the old format. We will have to regenerate the documentation (
yarn docs:build
).