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

[InputLabel] Remove FormLabelClasses in favor of asterisk class #14504

Merged
merged 6 commits into from
Feb 14, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions packages/material-ui/src/FormLabel/FormLabel.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export const styles = theme => ({
filled: {},
/* Styles applied to the root element if `required={true}`. */
required: {},
/* Styles applied to the asterisk element. */
asterisk: {
'&$error': {
color: theme.palette.error.main,
Expand Down
2 changes: 1 addition & 1 deletion packages/material-ui/src/InputLabel/InputLabel.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ export interface InputLabelProps extends StandardProps<FormLabelProps, InputLabe
disableAnimation?: boolean;
disabled?: boolean;
error?: boolean;
FormLabelClasses?: FormLabelProps['classes'];
focused?: boolean;
required?: boolean;
shrink?: boolean;
Expand All @@ -19,6 +18,7 @@ export type InputLabelClassKey =
| 'disabled'
| 'error'
| 'required'
| 'asterisk'
| 'formControl'
| 'marginDense'
| 'shrink'
Expand Down
11 changes: 4 additions & 7 deletions packages/material-ui/src/InputLabel/InputLabel.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ export const styles = theme => ({
error: {},
/* Styles applied to the root element if `required={true}`. */
required: {},
/* Styles applied to the asterisk element. */
asterisk: {},
/* Styles applied to the root element if the component is a descendant of `FormControl`. */
formControl: {
position: 'absolute',
Expand Down Expand Up @@ -86,7 +88,6 @@ function InputLabel(props) {
classes,
className,
disableAnimation,
FormLabelClasses,
Copy link
Member

Choose a reason for hiding this comment

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

This is a breaking change now.

margin,
muiFormControl,
shrink: shrinkProp,
Expand Down Expand Up @@ -125,7 +126,7 @@ function InputLabel(props) {
disabled: classes.disabled,
error: classes.error,
required: classes.required,
...FormLabelClasses,
asterisk: classes.asterisk,
}}
{...other}
>
Expand All @@ -141,7 +142,7 @@ InputLabel.propTypes = {
children: PropTypes.node,
/**
* Override or extend the styles applied to the component.
* See [CSS API](#css-api) below for more details.
* See [CSS API](#css) below for more details.
Copy link
Member

Choose a reason for hiding this comment

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

Nice finding!

Copy link
Member

Choose a reason for hiding this comment

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

We should fix all the other links (+100)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@oliviertassinari thanks for reviewing. Should I make separate PR for this?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, it would be awesome!

*/
classes: PropTypes.object.isRequired,
/**
Expand All @@ -164,10 +165,6 @@ InputLabel.propTypes = {
* If `true`, the input of this label is focused.
*/
focused: PropTypes.bool,
/**
* `classes` property applied to the [`FormLabel`](/api/form-label/) element.
*/
FormLabelClasses: PropTypes.object,
/**
* If `dense`, will adjust vertical spacing. This is normally obtained via context from
* FormControl.
Expand Down
8 changes: 4 additions & 4 deletions packages/material-ui/src/InputLabel/InputLabel.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,10 @@ describe('<InputLabel />', () => {
assert.strictEqual(wrapper.hasClass(classes.animated), false);
});

describe('prop: FormLabelClasses', () => {
it('should be able to change the FormLabel style', () => {
const wrapper = mount(<InputLabel FormLabelClasses={{ root: 'bar' }}>Foo</InputLabel>);
assert.include(wrapper.find('FormLabel').props().classes.root, 'bar');
describe('prop: classes', () => {
it('should be able to change the InputLabel style', () => {
const wrapper = mount(<InputLabel classes={{ root: 'bar' }}>Foo</InputLabel>);
assert.include(wrapper.find('InputLabel').props().classes.root, 'bar');
});
});

Expand Down
2 changes: 1 addition & 1 deletion packages/material-ui/test/typescript/components.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1048,7 +1048,7 @@ const PopoverTest = () => <Popover open ModalClasses={{ root: 'foo', hidden: 'ba

const InputLabelTest = () => (
<InputLabel
FormLabelClasses={{
classes={{
root: 'foo',
asterisk: 'foo',
Copy link
Member

Choose a reason for hiding this comment

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

@pelotom This was accepted before I added the asterisk to the InputLabelClassKey. Hovering over the classes prop did show Partial<Record<InputLabelClassKey, string>> which should reject that. Writing
const classes: Partial<Record<InputLabelClassKey, string>> = { asterisk: 'foo' } did however throw an error.

I suspect this is a typescript bug?

disabled: 'foo',
Expand Down
2 changes: 1 addition & 1 deletion pages/api/form-label.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ This property accepts the following keys:
| <span class="prop-name">error</span> | Styles applied to the root element if `error={true}`.
| <span class="prop-name">filled</span> | Styles applied to the root element if `filled={true}`.
| <span class="prop-name">required</span> | Styles applied to the root element if `required={true}`.
| <span class="prop-name">asterisk</span> |
| <span class="prop-name">asterisk</span> | Styles applied to the asterisk element.

Have a look at [overriding with classes](/customization/overrides/#overriding-with-classes) section
and the [implementation of the component](https://github.com/mui-org/material-ui/blob/master/packages/material-ui/src/FormLabel/FormLabel.js)
Expand Down
4 changes: 2 additions & 2 deletions pages/api/input-label.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,11 @@ import InputLabel from '@material-ui/core/InputLabel';
| Name | Type | Default | Description |
|:-----|:-----|:--------|:------------|
| <span class="prop-name">children</span> | <span class="prop-type">node</span> |   | The contents of the `InputLabel`. |
| <span class="prop-name">classes</span> | <span class="prop-type">object</span> |   | Override or extend the styles applied to the component. See [CSS API](#css-api) below for more details. |
| <span class="prop-name">classes</span> | <span class="prop-type">object</span> |   | Override or extend the styles applied to the component. See [CSS API](#css) below for more details. |
| <span class="prop-name">disableAnimation</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | If `true`, the transition animation is disabled. |
| <span class="prop-name">disabled</span> | <span class="prop-type">bool</span> |   | If `true`, apply disabled class. |
| <span class="prop-name">error</span> | <span class="prop-type">bool</span> |   | If `true`, the label will be displayed in an error state. |
| <span class="prop-name">focused</span> | <span class="prop-type">bool</span> |   | If `true`, the input of this label is focused. |
| <span class="prop-name">FormLabelClasses</span> | <span class="prop-type">object</span> |   | `classes` property applied to the [`FormLabel`](/api/form-label/) element. |
| <span class="prop-name">margin</span> | <span class="prop-type">enum:&nbsp;'dense'<br></span> |   | If `dense`, will adjust vertical spacing. This is normally obtained via context from FormControl. |
| <span class="prop-name">required</span> | <span class="prop-type">bool</span> |   | if `true`, the label will indicate that the input is required. |
| <span class="prop-name">shrink</span> | <span class="prop-type">bool</span> |   | If `true`, the label is shrunk. |
Expand All @@ -45,6 +44,7 @@ This property accepts the following keys:
| <span class="prop-name">disabled</span> | Styles applied to the root element if `disabled={true}`.
| <span class="prop-name">error</span> | Styles applied to the root element if `error={true}`.
| <span class="prop-name">required</span> | Styles applied to the root element if `required={true}`.
| <span class="prop-name">asterisk</span> | Styles applied to the asterisk element.
| <span class="prop-name">formControl</span> | Styles applied to the root element if the component is a descendant of `FormControl`.
| <span class="prop-name">marginDense</span> | Styles applied to the root element if `margin="dense"`.
| <span class="prop-name">shrink</span> | Styles applied to the `input` element if `shrink={true}`.
Expand Down