Skip to content

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

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Feb 14, 2019
Merged

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

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
07e9756
[InputLabel] Asteriks class for TextField
umairfarooq44 Feb 12, 2019
be3935c
reverted docs
umairfarooq44 Feb 12, 2019
a01d34e
[InputLabel] docs reverted
umairfarooq44 Feb 12, 2019
40adf5e
green build
oliviertassinari Feb 12, 2019
e3aff04
[InputLabel] Add asterisk class key
eps1lon Feb 13, 2019
994df13
Updated asterisk docs
umairfarooq44 Feb 14, 2019
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
View file
Open in desktop
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 if `required={true}`. */
Copy link
Member

@mbrookes mbrookes Feb 13, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Suggested change
/* Styles applied to the asterisk element if `required={true}`. */
/* Styles applied to the asterisk element. */

(required just controls whether the asterisk is displayed, not whether it receives this style.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for reviewing. I have added requested changes.

asterisk: {
'&$error': {
color: theme.palette.error.main,
Expand Down
1 change: 0 additions & 1 deletion packages/material-ui/src/InputLabel/InputLabel.d.ts
View file
Open in desktop
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 Down
11 changes: 4 additions & 7 deletions packages/material-ui/src/InputLabel/InputLabel.js
View file
Open in desktop
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 if `required={true}`. */
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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 if `required={true}`.

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
View file
Open in desktop
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 if `required={true}`.
| <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