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

[TextField] Migrate FormHelperText to emotion #24661

Merged
merged 21 commits into from
Jan 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
d84eafb
[TextField] Migrate FormHelperText to emotion
duganbrett Jan 27, 2021
e20b8f5
fixed variant
duganbrett Jan 27, 2021
c474211
Add docs
duganbrett Jan 27, 2021
c27dcdf
Add docs
duganbrett Jan 27, 2021
aa387c1
Update packages/material-ui/src/FormHelperText/formHelperTextClasses.…
mnajdova Jan 28, 2021
a725926
Update packages/material-ui/src/FormHelperText/formHelperTextClasses.js
mnajdova Jan 28, 2021
ba28881
Update packages/material-ui/src/FormHelperText/FormHelperText.js
mnajdova Jan 28, 2021
7c7b5e7
Update packages/material-ui/src/FormHelperText/FormHelperText.js
mnajdova Jan 28, 2021
0eee21a
Update packages/material-ui/src/FormHelperText/FormHelperText.js
mnajdova Jan 28, 2021
8d49960
Update packages/material-ui/src/FormHelperText/FormHelperText.js
mnajdova Jan 28, 2021
311bd54
Update packages/material-ui/src/FormHelperText/FormHelperText.js
mnajdova Jan 28, 2021
19e925b
Update packages/material-ui/src/FormHelperText/FormHelperText.js
mnajdova Jan 28, 2021
f98f030
Update packages/material-ui/src/FormHelperText/FormHelperText.js
mnajdova Jan 28, 2021
45d1443
Update packages/material-ui/src/FormHelperText/FormHelperText.js
mnajdova Jan 28, 2021
fe7a7e8
Update packages/material-ui/src/FormHelperText/FormHelperText.js
mnajdova Jan 28, 2021
76b721d
fixes
mnajdova Jan 28, 2021
935c276
try skipping JSDOM
mnajdova Jan 28, 2021
9824e08
Update packages/material-ui/src/FormHelperText/FormHelperText.js
mnajdova Jan 28, 2021
7b8124c
reverted tests
mnajdova Jan 28, 2021
ea88550
Merge branch 'migrate-form-helper-text' of https://github.com/duganbr…
mnajdova Jan 28, 2021
c05b91b
exported classes
mnajdova Jan 28, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion docs/pages/api-docs/form-helper-text.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
"focused": { "type": { "name": "bool" } },
"margin": { "type": { "name": "enum", "description": "'dense'" } },
"required": { "type": { "name": "bool" } },
"sx": { "type": { "name": "object" } },
"variant": {
"type": {
"name": "enum",
Expand Down Expand Up @@ -41,6 +42,6 @@
"filename": "/packages/material-ui/src/FormHelperText/FormHelperText.js",
"inheritance": null,
"demos": "<ul><li><a href=\"/components/text-fields/\">Text Fields</a></li></ul>",
"styledComponent": false,
"styledComponent": true,
"cssComponent": false
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
"focused": "If <code>true</code>, the helper text should use focused classes key.",
"margin": "If <code>dense</code>, will adjust vertical spacing. This is normally obtained via context from FormControl.",
"required": "If <code>true</code>, the helper text should use required classes key.",
"sx": "The system prop that allows defining system overrides as well as additional CSS styles. See the <a href=\"/system/basics/#the-sx-prop\">`sx` page</a> for more details.",
"variant": "The variant to use."
},
"classDescriptions": {
Expand Down
6 changes: 6 additions & 0 deletions packages/material-ui/src/FormHelperText/FormHelperText.d.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import * as React from 'react';
import { SxProps } from '@material-ui/system';
import { OverridableComponent, OverrideProps } from '../OverridableComponent';
import { Theme } from '../styles';

export interface FormHelperTextTypeMap<P = {}, D extends React.ElementType = 'p'> {
props: P & {
Expand Down Expand Up @@ -55,6 +57,10 @@ export interface FormHelperTextTypeMap<P = {}, D extends React.ElementType = 'p'
* If `true`, the helper text should use required classes key.
*/
required?: boolean;
/**
* The system prop that allows defining system overrides as well as additional CSS styles.
*/
sx?: SxProps<Theme>;
/**
* The variant to use.
*/
Expand Down
135 changes: 84 additions & 51 deletions packages/material-ui/src/FormHelperText/FormHelperText.js
Original file line number Diff line number Diff line change
@@ -1,52 +1,76 @@
import * as React from 'react';
import PropTypes from 'prop-types';
import clsx from 'clsx';
import { unstable_composeClasses as composeClasses } from '@material-ui/unstyled';
import { deepmerge } from '@material-ui/utils';
import formControlState from '../FormControl/formControlState';
import useFormControl from '../FormControl/useFormControl';
import withStyles from '../styles/withStyles';
import experimentalStyled from '../styles/experimentalStyled';
mnajdova marked this conversation as resolved.
Show resolved Hide resolved
import capitalize from '../utils/capitalize';
import { getFormHelperTextUtilityClasses } from './formHelperTextClasses';
import useThemeProps from '../styles/useThemeProps';

export const styles = (theme) => ({
/* Styles applied to the root element. */
root: {
color: theme.palette.text.secondary,
...theme.typography.caption,
textAlign: 'left',
marginTop: 3,
margin: 0,
'&$disabled': {
color: theme.palette.text.disabled,
},
'&$error': {
color: theme.palette.error.main,
},
const overridesResolver = (props, styles) => {
const { styleProps } = props;

return deepmerge(styles.root || {}, {
...(styleProps.size && styles[`size${capitalize(styleProps.size)}`]),
...(styleProps.contained && styles.contained),
...(styleProps.filled && styles.filled),
});
};

const useUtilityClasses = (styleProps) => {
const { classes, contained, size, disabled, error, filled, focused, required } = styleProps;
const slots = {
root: [
'root',
disabled && 'disabled',
error && 'error',
size && `size${capitalize(size)}`,
contained && 'contained',
focused && 'focused',
filled && 'filled',
required && 'required',
],
};

return composeClasses(slots, getFormHelperTextUtilityClasses, classes);
};

const FormHelperTextRoot = experimentalStyled(
'p',
{},
{ name: 'MuiFormHelperText', slot: 'Root', overridesResolver },
)(({ theme, styleProps }) => ({
color: theme.palette.text.secondary,
...theme.typography.caption,
textAlign: 'left',
marginTop: 3,
marginRight: 0,
marginBottom: 0,
marginLeft: 0,
'&.Mui-disabled': {
color: theme.palette.text.disabled,
},
/* Pseudo-class applied to the root element if `error={true}`. */
error: {},
/* Pseudo-class applied to the root element if `disabled={true}`. */
disabled: {},
/* Styles applied to the root element if `size="small"`. */
sizeSmall: {
marginTop: 4,
'&.Mui-error': {
color: theme.palette.error.main,
},
/* Styles applied to the root element if `variant="filled"` or `variant="outlined"`. */
contained: {
...(styleProps.size === 'small' && {
marginTop: 4,
}),
...(styleProps.contained && {
marginLeft: 14,
marginRight: 14,
},
/* Pseudo-class applied to the root element if `focused={true}`. */
focused: {},
/* Pseudo-class applied to the root element if `filled={true}`. */
filled: {},
/* Pseudo-class applied to the root element if `required={true}`. */
required: {},
});
}),
}));

const FormHelperText = React.forwardRef(function FormHelperText(props, ref) {
const FormHelperText = React.forwardRef(function FormHelperText(inProps, ref) {
const props = useThemeProps({ props: inProps, name: 'MuiFormHelperText' });
const {
children,
classes,
className,
component: Component = 'p',
component = 'p',
disabled,
error,
filled,
Expand All @@ -64,21 +88,26 @@ const FormHelperText = React.forwardRef(function FormHelperText(props, ref) {
states: ['variant', 'size', 'disabled', 'error', 'filled', 'focused', 'required'],
});

const styleProps = {
...props,
mnajdova marked this conversation as resolved.
Show resolved Hide resolved
component,
contained: fcs.variant === 'filled' || fcs.variant === 'outlined',
variant: fcs.variant,
size: fcs.size,
disabled: fcs.disabled,
error: fcs.error,
filled: fcs.filled,
focused: fcs.focused,
required: fcs.required,
};

const classes = useUtilityClasses(styleProps);

return (
<Component
className={clsx(
classes.root,
{
[classes.contained]: fcs.variant === 'filled' || fcs.variant === 'outlined',
[classes.sizeSmall]: fcs.size === 'small',
[classes.disabled]: fcs.disabled,
[classes.error]: fcs.error,
[classes.filled]: fcs.filled,
[classes.focused]: fcs.focused,
[classes.required]: fcs.required,
},
className,
)}
<FormHelperTextRoot
as={component}
styleProps={styleProps}
className={clsx(classes.root, className)}
ref={ref}
{...other}
>
Expand All @@ -89,7 +118,7 @@ const FormHelperText = React.forwardRef(function FormHelperText(props, ref) {
) : (
children
)}
</Component>
</FormHelperTextRoot>
);
});

Expand Down Expand Up @@ -142,10 +171,14 @@ FormHelperText.propTypes = {
* If `true`, the helper text should use required classes key.
*/
required: PropTypes.bool,
/**
* The system prop that allows defining system overrides as well as additional CSS styles.
*/
sx: PropTypes.object,
/**
* The variant to use.
*/
variant: PropTypes.oneOf(['filled', 'outlined', 'standard']),
};

export default withStyles(styles, { name: 'MuiFormHelperText' })(FormHelperText);
export default FormHelperText;
13 changes: 6 additions & 7 deletions packages/material-ui/src/FormHelperText/FormHelperText.test.js
Original file line number Diff line number Diff line change
@@ -1,24 +1,23 @@
import * as React from 'react';
import { expect } from 'chai';
import { getClasses, createMount, createClientRender, describeConformance } from 'test/utils';
import { createMount, createClientRender, describeConformanceV5 } from 'test/utils';
import FormHelperText from './FormHelperText';
import FormControl from '../FormControl';
import classes from './formHelperTextClasses';

describe('<FormHelperText />', () => {
const mount = createMount();
const render = createClientRender();
let classes;

before(() => {
classes = getClasses(<FormHelperText />);
});

describeConformance(<FormHelperText />, () => ({
describeConformanceV5(<FormHelperText />, () => ({
classes,
inheritComponent: 'p',
mount,
refInstanceof: window.HTMLParagraphElement,
testComponentPropWith: 'div',
muiName: 'MuiFormHelperText',
testVariantProps: { size: 'small' },
skip: ['componentsProp'],
}));

describe('prop: error', () => {
Expand Down
17 changes: 17 additions & 0 deletions packages/material-ui/src/FormHelperText/formHelperTextClasses.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
export interface FormHelperTextClasses {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A question while I'm here: Do these need to be interfaces for module augmentation or would a Record<FormHelperTextClassKey, string> be sufficient?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good point, I believe we can use the same. It will also ensure that the typings on the classses are correct.__

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder how we should best handle this. So far the components were re-introducing this new typing. In order not to block the migration, should we handle this change in the end for all components once they are migrated? I will leave a comment on the issue for this change.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that we could try to migrate one component to this approach, and if it works well, migrate them all at once? I could do it this weekend if needed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me 👍 will merge this one then and we have this #24405 (comment) for reference of what needs to be done

root: string;
error: string;
disabled: string;
sizeSmall: string;
mnajdova marked this conversation as resolved.
Show resolved Hide resolved
sizeMedium: string;
contained: string;
focused: string;
filled: string;
required: string;
}

declare const formHelperTextClasses: FormHelperTextClasses;

export function getFormHelperTextUtilityClasses(slot: string): string;

export default formHelperTextClasses;
19 changes: 19 additions & 0 deletions packages/material-ui/src/FormHelperText/formHelperTextClasses.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { generateUtilityClass, generateUtilityClasses } from '@material-ui/unstyled';

export function getFormHelperTextUtilityClasses(slot) {
return generateUtilityClass('MuiFormHelperText', slot);
}

const formHelperTextClasses = generateUtilityClasses('MuiFormHelperText', [
'root',
'error',
'disabled',
'sizeSmall',
mnajdova marked this conversation as resolved.
Show resolved Hide resolved
'sizeMedium',
'contained',
'focused',
'filled',
'required',
]);

export default formHelperTextClasses;
3 changes: 3 additions & 0 deletions packages/material-ui/src/FormHelperText/index.d.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,5 @@
export { default } from './FormHelperText';
export * from './FormHelperText';

export { default as formHelperTextClasses } from './formHelperTextClasses';
export * from './formHelperTextClasses';
3 changes: 3 additions & 0 deletions packages/material-ui/src/FormHelperText/index.js
Original file line number Diff line number Diff line change
@@ -1 +1,4 @@
export { default } from './FormHelperText';

export { default as formHelperTextClasses } from './formHelperTextClasses';
export * from './formHelperTextClasses';