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

[Dialog] The h6 DialogTitle class is applied to dialog titles instead of h2 #26386

Closed
2 tasks done
Anna-log7 opened this issue May 19, 2021 · 5 comments
Closed
2 tasks done
Labels
component: dialog This is the name of the generic UI component, not the React module! design: material This is about Material Design, please involve a visual or UX designer in the process support: question Community support but can be turned into an improvement

Comments

@Anna-log7
Copy link

Anna-log7 commented May 19, 2021

The documentation and code both indicate that an h2 should be the default variant for DialogTitle, but the h6 class is applied.
This line of code has the component set to h2, but the variant set as h6.

https://github.com/mui-org/material-ui/blob/7df94e4235fc6fc98ec7c13710be45bdbbd86dbe/packages/material-ui/src/DialogTitle/DialogTitle.js#L24

This results in <h2 class="MuiTypography-root MuiTypography-h6"></h2>

Also see this related closed issue: #16569

  • 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 😯

.MuiTypography-h6 class is set on the DialogTitle

Expected Behavior 🤔

.MuiTypography-h2 class should be set on the DialogTitle

Steps to Reproduce 🕹

https://codesandbox.io/s/h6-bug-323pj?file=/demo.js

Context 🔦

Our h6 styling is much smaller causing the dialog titles to not stand out as expected.

Your Environment 🌎

`npx @material-ui/envinfo`
  System:
    OS: Linux 3.10 CentOS Linux 7 (Core)
  Binaries:
    Node: 12.20.1 - ~/.nvm/versions/node/v12.20.1/bin/node
    Yarn: Not Found
    npm: 6.14.10 - ~/.nvm/versions/node/v12.20.1/bin/npm
  Browsers:
    Chrome: 90.0.4430.72
    Firefox: 88.0
  npmPackages:
    @material-ui/core: ~4.11.3 => 4.11.3 
    @material-ui/icons: ~4.5.1 => 4.5.1 
    @material-ui/lab: ~4.0.0-alpha.44 => 4.0.0-alpha.44 
    @material-ui/styles: ~4.9.13 => 4.9.13 
    @material-ui/system:  4.11.3 
    @material-ui/types:  5.1.0 
    @material-ui/utils:  4.7.1 
    @types/react: ~16.9.47 => 16.9.47 
    react: ~16.9.0 => 16.9.0 
    react-dom: ~16.9.0 => 16.9.0 
    typescript: ~4.0.2 => 4.0.2.
@Anna-log7 Anna-log7 added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label May 19, 2021
@mnajdova
Copy link
Member

After reading trough the discussion, I think that we should replace the variant to correspond to the tag used:

index 8837d98d39..817595e0f3 100644
--- a/packages/material-ui/src/DialogTitle/DialogTitle.js
+++ b/packages/material-ui/src/DialogTitle/DialogTitle.js
@@ -50,7 +50,7 @@ const DialogTitle = React.forwardRef(function DialogTitle(inProps, ref) {
       {disableTypography ? (
         children
       ) : (
-        <Typography component="h2" variant="h6">
+        <Typography component="h2" variant="h2">
           {children}
         </Typography>
       )}

Tagging @oliviertassinari as he was leading the initial discussion.

@mnajdova mnajdova added breaking change component: dialog This is the name of the generic UI component, not the React module! design: material This is about Material Design, please involve a visual or UX designer in the process and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels May 21, 2021
@oliviertassinari oliviertassinari added support: question Community support but can be turned into an improvement and removed breaking change labels May 21, 2021
@oliviertassinari
Copy link
Member

oliviertassinari commented May 21, 2021

h6 is the correct typography variant, per https://material.io/components/dialogs#simple-dialog.

The documentation and code both indicate that an h2 should be the default variant for DialogTitle, but the h6 class is applied.

@Anna-log7 Do you have material to support this claim? More importantly, what's your root pain point? What problem are you trying to solve?

This issue might be a bit related to #19696.

@oliviertassinari oliviertassinari changed the title [DialogTitle] The h6 class is applied to dialog titles instead of h2 [Dialog] The h6 DialogTitle class is applied to dialog titles instead of h2 May 21, 2021
@Anna-log7
Copy link
Author

h6 is the correct typography variant, per https://material.io/components/dialogs#simple-dialog.

From what I can tell after reading that page, h6 isn't specified as the "correct" heading for a dialog though it is used in the Reply Material Theme and the Crane travel app theme as theming examples.

The documentation and code both indicate that an h2 should be the default variant for DialogTitle, but the h6 class is applied.

@Anna-log7 Do you have material to support this claim? More importantly, what's your root pain point? What problem are you trying to solve?

This page in the documentation https://material-ui.com/api/dialog-title/ lists h2 as the default typography variant for DialogTitle when it's not the case in reality.
image
My main issue is I want our application to use an h2 to match the rest of our styling for dialogs. I could use an override to solve this (and I am currently) but I thought I'd make this ticket to point out the issue for others.

@oliviertassinari
Copy link
Member

Interesting, the h2 in the API description is a reference to the DOM node. The pain you have seem very much something we aim to fix in #19696. And if I understand it correctly, you want to customize the typography variant of the DialogTitle in the theme.

@oliviertassinari
Copy link
Member

oliviertassinari commented May 31, 2021

Fixed in #26323

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: dialog This is the name of the generic UI component, not the React module! design: material This is about Material Design, please involve a visual or UX designer in the process support: question Community support but can be turned into an improvement
Projects
None yet
Development

No branches or pull requests

3 participants