[DialogTitle] Documentation default variant different from default rendered

[dayander]

In the documentation the default component rendered for DialogTilte is an h2 but an h6 is the default rendered element.

• This is not a v0.x issue.
• I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior 🤔

Based on documentation the default element rendered for DialogTitle is an h2 element.

Current Behavior 😯

The default element rendered is an h6

Steps to Reproduce 🕹

Link:

1. https://codesandbox.io/s/create-react-app-ns87j
2. examples on https://material-ui.com/components/dialogs/

Context 🔦

To make our application more accessible, while also making it easier for people to contribute without a large knowledge base of accessibility.

Your Environment 🌎

Tech	Version
Material-UI	v4.1.1
React	v 16.8.6
Browser	Chrome v75.0
etc.

[oliviertassinari]

@dayander Do you mean that we should change the description from default h2 to h6?

material-ui/packages/material-ui/src/DialogTitle/DialogTitle.js

Line 42 in 76b7b7e

* For instance, this can be useful to render an h4 instead of the default h2.

[dayander]

@oliviertassinari I think the default should be an h2 and match the documentation. Rendering as an h2 is best practice for accessibility.

[oliviertassinari]

Do you have resources on why h2?

[oliviertassinari]

• https://www.w3.org/TR/wai-aria-practices/examples/dialog-modal/dialog.html uses h2
• https://www.youtube.com/watch?v=JS68faEUduk uses h2
• https://getbootstrap.com/docs/4.3/components/modal/ uses h5
• https://allyjs.io/tutorials/accessible-dialog.html#interactive-demo uses h1

Does the actual header used matter?

[dayander]

• https://www.w3.org/TR/wai-aria-practices/examples/dialog-modal/dialog.html
  WAI-ARIA is one of the working groups that decide accessibility standards. I think it's best practice to follow what they do.

Google owns Youtube and has people contributing to the working group for WAI-ARIA.

Bootstrap is not a source for accessibility.

Each page should contain one h1 denoting the whole meaning of the page. The next consecutive order after an h1 is h2. When the dialog becomes an overlay over the current page and should still fall under the content scope of the h1 and the next heading level should be an h2.

[oliviertassinari]

Each page should contain one h1 denoting the whole meaning of the page. The next consecutive order after an h1 is h2. When the dialog becomes an overlay over the current page and should still fall under the content scope of the h1 and the next heading level should be an h2.

@dayander Is this detailed somewhere? What's the impact? I would love to learn more about it.

Bootstrap is not a source for accessibility.

Agree, it's not a source of truth.
It's not all that bad. Bootstrap has https://github.com/patrickhlauke as a contributor, he has done more than 200 commits one topics that seem related to accessibility, he is a member of the WCAG.

cc @eps1lon

[Forfold]

Hey, I was just chatting with @dayander a little bit about this and thought I could chime in with a few more resources to look at.

The higher the ranking of the header, the less important it is on the page. A heading element should never exceed h6. Although theoretically you can go higher, and some screen readers may support it, the results can be unpredictable with other browser/screen reader combinations. [1]

If the dialog header begins at h6, this can cause issues for screen readers if any other headers need to be added to the same dialog, as they would have to be an h7 or higher. [2]

Another resource for this information can be found here: https://www.w3.org/WAI/tutorials/page-structure/headings/

1: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/heading_role#Accessibility_concerns
2: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/Heading_Elements#Accessibility_concerns

[oliviertassinari]

@Auchindoun Thanks, that sounds good to me.

The change could be:

diff --git a/docs/src/pages/components/modal/modal.md b/docs/src/pages/components/modal/modal.md
index 4299aa417..bf1c98774 100644
--- a/docs/src/pages/components/modal/modal.md
+++ b/docs/src/pages/components/modal/modal.md
@@ -88,9 +88,9 @@ Additionally, you may give a description of your modal with the `aria-describedb
   aria-labelledby="simple-modal-title"
   aria-describedby="simple-modal-description"
 >
-  <h6 id="modal-title">
+  <h2 id="modal-title">
     My Title
-  </h6>
+  </h2>
   <p id="simple-modal-description">
     My Description
   </p>
diff --git a/packages/material-ui/src/DialogTitle/DialogTitle.js b/packages/material-ui/src/DialogTitle/DialogTitle.js
index f3cc523ce..e877b67b4 100644
--- a/packages/material-ui/src/DialogTitle/DialogTitle.js
+++ b/packages/material-ui/src/DialogTitle/DialogTitle.js
@@ -18,7 +18,7 @@ const DialogTitle = React.forwardRef(function DialogTitle(props, ref) {

   return (
     <div className={clsx(classes.root, className)} ref={ref} {...other}>
-      {disableTypography ? children : <Typography variant="h6">{children}</Typography>}
+      {disableTypography ? children : <Typography component="h2" variant="h6">{children}</Typography>}
     </div>
   );
 });

[dayander]

That sounds great. Thank you. I can make the changes and open a PR.

[shiraze]

Why is the h2 component given a class that renders it with h6 typography?
<h2 class="MuiTypography-root MuiTypography-h6">Dialog Title text</h2>

[oliviertassinari]

@shiraze I don't understand. What do you mean?

[shiraze]

@oliviertassinari , sorry I just re-read what I said and it's not clear!
I have jsx with <DialogTitle>Dialog Title text</DialogTitle> which renders in the browser as <div class="MuiDialogTitle-root"><h2 class="MuiTypography-root MuiTypography-h6">Dialog Title text</h2></div>

The inner component is an h2 but the css class applied to it (MuiTypography-h6) renders it as an h6

[oliviertassinari]

@shiraze Thanks for the details. Yes, it's the expected behavior. Does https://material-ui.com/components/typography/#changing-the-semantic-element make it clearer?

[ghan]

what's the cleanest way to override this h6 variant and use h2 instead, from the global overrides object? thanks

[oliviertassinari]

@ghan Upgrade to v4 latest or target DialogTitle with https://material-ui.com/customization/globals/#default-props.

[ghan]

Hi Oliver, thanks for responding. My question was a little vague, I apologize. I'm on v4.5 and want to have <h2 variant="h2" />, not <h2 variant="h6" /> globally. I've added my current workaround... I'm hoping there's a cleaner solution out there:

First disable the nested h2 element:

  props: {
        MuiDialogTitle: {
            disableTypography: true
        }
    }

Then apply h2 styles to the DialogTitle component via overrides:

overrides: {
        MuiDialogTitle: {
            root: {
                fontFamily: headerFont,
                fontSize: headerSize,
                fontWeight: headerWeight,
                lineHeight: headerLineheight
            }
        }
    }

I hope that demonstrates a little better why this feels sub-optimal to me. Let me know if you guys do something smarter, thanks!

[oliviertassinari]

@ghan I'm sorry, It still unclear what's your objective, I would suggest to try StackOverflow.

[KyruCabading]

Hi @oliviertassinari, regarding @ghan 's use case. I actually came across the very exact issue.
Let me try explaining the issue in a different way.

Although the DialogTitle allows us to customize the Typography component used within DialogType through disableTypography (like to change the rendered variant h6 to h3), the current API does not allow us to change all DialogTitle's Typography component globally due to hardcoding in this line:
https://github.com/mui-org/material-ui/blob/master/packages/material-ui/src/DialogTitle/DialogTitle.js#L24...

If we wanted to have all DialogTitles rendering H3's, we would have to disableTypography in every DialogTitle throughout our apps...

A possible solution might be to add a prop to DialogTitle called 'variant' which maps to
<Typography component="h2" variant={variant}>
And just default with h6... Then make sure the global theme api can used to override it.

theme = {
    props: {
        DialogTitle: {
            variant: 'h3' <--- Or anything required
        },
}

.... I came across this issue myself. I thought it was possible through some sort of api like

theme = {
    props: {
        DialogTitle: {
            children: props => <Typography component="h2" variant="h3">{props.children}</Typography>
        },
}

But that's not the case! :D For now I have to disableTypography in all our DialogTitles because ours requires to use the actual H2 font style.

I can try to clarify it again if needed!

[Wagan8r]

Yes, I am also running into this issue. @KyruCabading proposed solution is exactly what I would have expected this component to support from the get-go. This would be a very useful update!