Theme -> typography -> caption -> lineHeight is ignored

[zsolt-dev]

• The issue is present in the latest release.
• I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

Theme -> typography -> caption -> lineHeight is ignored

Expected Behavior 🤔

Theme -> typography -> caption -> lineHeight should not be ignored :)

Steps to Reproduce 🕹

1. open: https://codesandbox.io/s/naughty-sun-0k7jb
2. inspect the provided FormHelperText
3. lineHeight is hardcoded and value from the theme is ignored

Latest stable material-ui and latest stable react

Thank you

[oliviertassinari]

Interesting. Yeah, I guess we can rely on the caption typography directly. What do you think of this diff?

diff --git a/packages/material-ui/src/FormHelperText/FormHelperText.js b/packages/material-ui/src/FormHelperText/FormHelperText.js
index c0da0a8e9..76660a69a 100644
--- a/packages/material-ui/src/FormHelperText/FormHelperText.js
+++ b/packages/material-ui/src/FormHelperText/FormHelperText.js
@@ -11,9 +11,7 @@ export const styles = theme => ({
     color: theme.palette.text.secondary,
     ...theme.typography.caption,
     textAlign: 'left',
-    marginTop: 8,
-    lineHeight: '1em',
-    minHeight: '1em',
+    marginTop: 4,
     margin: 0,
     '&$disabled': {
       color: theme.palette.text.disabled,
@@ -32,7 +30,8 @@ export const styles = theme => ({
   },
   /* Styles applied to the root element if `variant="filled"` or `variant="outlined"`. */
   contained: {
-    margin: '8px 14px 0',
+    marginLeft: 14,
+    marginRight: 14,
   },
   /* Pseudo-class applied to the root element if `focused={true}`. */
   focused: {},

[zsolt-dev]

@oliviertassinari Looks great! Thank you for a quick response and coming up with the solution.

[oliviertassinari]

Great, do you want to work on the issue?

[zsolt-dev]

Thank you, but I do not understand material-ui so deeply and also do not know typescript.

Also you already did the work, so you deserve the credit for it.

[suliskh]

Has anyone assigned to it? I am happy to help! :D

[TommyJackson85]

@suliskh @oliviertassinari if neither of you assigned your self to this issue please let me know. I am willing to do this as my first issue attempt for this repository.

[oliviertassinari]

Note that the proposed patch seems to close #19359 at the same time :), but to be confirmed.

[suliskh]

Okay, I am going to work on the proposed patch.

[suliskh]

Interesting. Yeah, I guess we can rely on the caption typography directly. What do you think of this diff?

diff --git a/packages/material-ui/src/FormHelperText/FormHelperText.js b/packages/material-ui/src/FormHelperText/FormHelperText.js
index c0da0a8e9..76660a69a 100644
--- a/packages/material-ui/src/FormHelperText/FormHelperText.js
+++ b/packages/material-ui/src/FormHelperText/FormHelperText.js
@@ -11,9 +11,7 @@ export const styles = theme => ({
     color: theme.palette.text.secondary,
     ...theme.typography.caption,
     textAlign: 'left',
-    marginTop: 8,
-    lineHeight: '1em',
-    minHeight: '1em',
+    marginTop: 4,
     margin: 0,
     '&$disabled': {
       color: theme.palette.text.disabled,
@@ -32,7 +30,8 @@ export const styles = theme => ({
   },
   /* Styles applied to the root element if `variant="filled"` or `variant="outlined"`. */
   contained: {
-    margin: '8px 14px 0',
+    marginLeft: 14,
+    marginRight: 14,
   },
   /* Pseudo-class applied to the root element if `focused={true}`. */
   focused: {},

The [material-design] is using pseudo-element with specified height to create gap between text field and the form text helper. The computed height of the form text helper is 19px, and the height of the pseudo-element is 16px, so I am gonna go with 3px gap.

@oliviertassinari