-
-
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 12 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 |
---|---|---|
@@ -0,0 +1,15 @@ | ||
FormHelperText | ||
========= | ||
|
||
|
||
|
||
Props | ||
----- | ||
|
||
| Name | Type | Default | Description | | ||
|:-----|:-----|:--------|:------------| | ||
| children | node | | The content of the component. | | ||
| 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. |
This file was deleted.
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 |
---|---|---|
@@ -0,0 +1,70 @@ | ||
// @flow weak | ||
/* eslint-disable jsx-a11y/label-has-for */ | ||
|
||
import React from 'react'; | ||
import PropTypes from 'prop-types'; | ||
import classNames from 'classnames'; | ||
import { createStyleSheet } from 'jss-theme-reactor'; | ||
import customPropTypes from '../utils/customPropTypes'; | ||
|
||
export const styleSheet = createStyleSheet('MuiFormHelperText', (theme) => { | ||
return { | ||
root: { | ||
color: theme.palette.input.helperText, | ||
fontSize: 12, | ||
lineHeight: 1.4, | ||
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. Wouldn't 1.3' round to 1.3? 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. 1.3 results to 15px line-height in webkit... 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. Gotcha. |
||
margin: 0, | ||
}, | ||
error: { | ||
color: theme.palette.error.A400, | ||
}, | ||
}; | ||
}); | ||
|
||
export default function FormHelperText(props, context) { | ||
const { | ||
children, | ||
className: classNameProp, | ||
error: errorProp, | ||
...other | ||
} = props; | ||
|
||
const { muiFormControl, styleManager } = context; | ||
const classes = styleManager.render(styleSheet); | ||
|
||
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, | ||
/** | ||
* 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, | ||
styleManager: customPropTypes.muiRequired, | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,7 +6,7 @@ import classNames from 'classnames'; | |
import { createStyleSheet } from 'jss-theme-reactor'; | ||
import customPropTypes from '../utils/customPropTypes'; | ||
|
||
function isDirty(obj) { | ||
export function isDirty(obj) { | ||
return obj && obj.value && obj.value.length > 0; | ||
} | ||
|
||
|
@@ -16,15 +16,16 @@ export const styleSheet = createStyleSheet('MuiInput', (theme) => { | |
wrapper: { | ||
// Mimics the default input display property used by browsers for an input. | ||
display: 'inline-block', | ||
marginBottom: 8, | ||
marginTop: 8, | ||
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. Hum, why changing that? The less our component impact the layout, the better. |
||
position: 'relative', | ||
}, | ||
formControl: { | ||
marginTop: 10, | ||
marginBottom: 10, | ||
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? |
||
}, | ||
inkbar: { | ||
'&:after': { | ||
backgroundColor: palette.primary.A200, | ||
backgroundColor: palette.type === 'light' ? palette.primary.A700 : palette.primary.A200, | ||
left: 0, | ||
bottom: -1, | ||
// Doing the other way around crash on IE11 "''" https://github.com/cssinjs/jss/issues/242 | ||
|
@@ -45,7 +46,7 @@ export const styleSheet = createStyleSheet('MuiInput', (theme) => { | |
focused: {}, | ||
error: { | ||
'&:after': { | ||
backgroundColor: palette.error[500], | ||
backgroundColor: palette.error.A400, | ||
transform: 'scaleX(1)', // error is always underlined in red | ||
}, | ||
}, | ||
|
@@ -59,7 +60,7 @@ export const styleSheet = createStyleSheet('MuiInput', (theme) => { | |
background: 'none', | ||
lineHeight: 1, | ||
appearance: 'textfield', // Improve type search style. | ||
color: theme.palette.text.primary, | ||
color: theme.palette.input.inputText, | ||
width: '100%', | ||
'&:focus': { | ||
outline: 0, | ||
|
@@ -69,11 +70,19 @@ export const styleSheet = createStyleSheet('MuiInput', (theme) => { | |
}, | ||
}, | ||
disabled: { | ||
color: theme.palette.text.disabled, | ||
color: theme.palette.input.disabled, | ||
cursor: 'not-allowed', | ||
}, | ||
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: -1, | ||
transition: transitions.create('border-color', { | ||
duration: transitions.duration.shorter, | ||
easing: transitions.easing.ease, | ||
}), | ||
}, | ||
'&$disabled': { | ||
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 think that our convention is the following |
||
borderBottomStyle: 'dotted', | ||
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 don't think we need that style. |
||
}, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,28 +12,34 @@ export const styleSheet = createStyleSheet('MuiInputLabel', (theme) => { | |
return { | ||
root: { | ||
transformOrigin: 'top left', | ||
fontSmoothing: 'antialiased', | ||
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. Ohh, that's a tough topic! I think that it far out of the scope of this PR.
|
||
}, | ||
formControl: { | ||
position: 'absolute', | ||
left: 0, | ||
top: 0, | ||
transform: 'translate(0, 18px) scale(1)', | ||
transform: 'translate(0, 40px)', | ||
}, | ||
shrink: { | ||
transform: 'translate(0, 0px) scale(0.75)', | ||
fontSize: 12, | ||
transform: 'translate(0, 18px)', | ||
transformOrigin: 'top left', | ||
}, | ||
animated: { | ||
transition: transitions.create('transform', { | ||
transition: transitions.create(['transform', 'font-size'], { | ||
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. Hum, I see two advantages of animating the scale:
|
||
duration: transitions.duration.shorter, | ||
easing: transitions.easing.easeOut, | ||
}), | ||
}, | ||
disabled: { | ||
color: theme.palette.input.disabled, | ||
}, | ||
}; | ||
}); | ||
|
||
export default function InputLabel(props, context) { | ||
const { | ||
disabled, | ||
disableAnimation, | ||
children, | ||
className: classNameProp, | ||
|
@@ -54,6 +60,7 @@ export default function InputLabel(props, context) { | |
[classes.formControl]: muiFormControl, | ||
[classes.animated]: !disableAnimation, | ||
[classes.shrink]: shrink, | ||
[classes.disabled]: disabled, | ||
}, classNameProp); | ||
|
||
return ( | ||
|
@@ -76,6 +83,10 @@ InputLabel.propTypes = { | |
* If `true`, the transition animation is disabled. | ||
*/ | ||
disableAnimation: PropTypes.bool, | ||
/** | ||
* If `true`, apply disabled 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. 'If Hmmm, now I wonder if all the others should follow that pattern. It's clearer to the user what the outcome of applying this prop is when it comes to custom styling. |
||
*/ | ||
disabled: PropTypes.bool, | ||
/** | ||
* If `true`, the label will be displayed in an error state. | ||
*/ | ||
|
@@ -95,6 +106,7 @@ InputLabel.propTypes = { | |
}; | ||
|
||
InputLabel.defaultProps = { | ||
disabled: false, | ||
disableAnimation: false, | ||
}; | ||
|
||
|
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
).