MudText: Replace h6 with span for subtitle typos (#6059)

[tpmccrary]

Instead of initializing the text with <h6> tag for Typo.subtitle1 and Typo.subtitle2, initializes it with the more generic inline container: <span> tag.

Description

Resolves #6059
When using Typo.subtitle2 on MudText, the text is not centered vertically, causing it to look strange with all other vertically centered text.
After further investigation with inspect element, it turns out that the text is being initialized using the tag <h6>, causing this inconsistency.

How Has This Been Tested?

Visually

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)

The text "The University of Somewhere" with Typo.subtitle2 using <h6>:

The text "The University of Somewhere" with Typo.subtitle2 using <span>:

Direct comparison. <h6> on the left, <span> on the right.

Checklist:

• The PR is submitted to the correct branch (dev).
• My code follows the code style of this project.
• I've added relevant tests.

[codecov]

Codecov Report

Base: 91.52% // Head: 91.53% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (34f9237) compared to base (a6116c6).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

@@           Coverage Diff           @@
##              dev    #6061   +/-   ##
=======================================
  Coverage   91.52%   91.53%           
=======================================
  Files         383      383           
  Lines       15019    15019           
=======================================
+ Hits        13746    13747    +1     
+ Misses       1273     1272    -1     

Impacted Files	Coverage Δ
src/MudBlazor/Components/Typography/MudText.razor	100.00% <100.00%> (ø)
...r/Components/Autocomplete/MudAutocomplete.razor.cs	92.85% <0.00%> (+0.42%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[mikes-gh]

Not sure about this. Because it is visually breaking. It solves a specific but is it a good idea for all scenarios?

@JonBunator

[erikemtz]

Seems like span used to be used too

[dennisrahmen]

For me at least it's annoying every time I see the text not aligned properly.

Maybe it could be included in the 7.x version with the other breaking changes?

[JonBunator]

How are you centering the text?

[tpmccrary]

How are you centering the text?

The text becomes vertically centered when I change it to use the <span> tag.

[lortimer]

I encountered this bug while using MudBlazor today and I came here to make sure there was a plan to address it. I want to answer @mikes-gh 's question above:

Not sure about this. Because it is visually breaking. It solves a specific but is it a good idea for all scenarios?

This is a very important issue to fix for accessibility purposes for all scenarios. People who use assistive technology often navigate pages by the heading elements on the page. Mobile and desktop screen readers can list and read the headings on a page to help a user find the section they want without reading the whole page top to bottom. The h1 through h6 elements should only be used to create this kind of semantic structure in a document, not for styling text.

When developers use the Typo.subtitle1 and Typo.subtitle2 typographies, they are expecting to style the text as a subtitle - they are not expecting to create or change any semantic structure. The fact that these elements create an h6 on the page creates confusion for people using assistive technologies. Developers using MudBlazor may inadvertently fail to meet WCAG 2.1 success criteria 1.3.1 and 2.4.10, which they may be required to meet.

At a practical level this bug could create a scenario like the example below without the developers realizing it.

<h1>Main Heading</h1>
<h2>Topic 1</h2>
...Some content here
<h6>This is supposed to be text styled as a subtitle, but is now a misplaced heading</h6>
<h3>Subtopic 1</h3>

This could be confusing for users, because now the h3 for subtopic 1 is no longer semantically linked to topic 1.

I was not able to reproduce the vertical alignment issue mentioned above, but it doesn't surprise me because span is an inline element and the heading elements are block elements. I would suggest using the p element instead of span because p is also a block element. body1 and body2 already use p. I'm happy to submit a PR for that, but I don't know how to reproduce the vertical alignment issue. Maybe @tpmccrary could try using a p instead and see if the vertical alignment issue persists.

I hope you can find a way to fix and release this as soon as possible because of the potential impact this has on the accessibility of web apps using MudBlazor.

[dennisrahmen]

I would root for replacing the <h6> with <span> and merging this with all the other breaking changes for v7.
That would fix the alignment as well as the screen reader issue.

[lortimer]

@dennisrahmen When is version 7 going to be released? Any reason not to even try using p instead so it could be released in some v6 version? Releasing it in v6 would make the fix available to a lot of apps that will never upgrade to v7.

MudBlazor is an authoring tool and as such has take accessibility seriously. It should make creating accessible web interfaces easier and avoid creating accessibility issues that developers might miss.

[dennisrahmen]

@lortimer the last thing I read, from I think @henon, is that the v7 release is already overdue.

Regarding:

Releasing it in v6 would make the fix available to a lot of apps that will never upgrade to v7.

If they are not upgrading to v7 then they also might not upgrade to v6.x, as v7 does only include minor breaking changes compared to most other libraries I use. Thats my opinion, I am just a user here.

[ScarletKuro]

@henon should we add this to v7 project?

[henon]

I changed the target branch to v7. Are there any other accesibility changes that should be made in MudText or is everything else fine?

[danielchalmers]

I changed the target branch to v7. Are there any other accesibility changes that should be made in MudText or is everything else fine?

Target branch can be changed back to dev @henon

[henon]

Thank you for the patience, I know it has been a long wait. v7.0.0 is finally being actively worked on. All breaking changes are summarized in the v7.0.0 Migration Guide #8447

[ScarletKuro]

This PR broke some tests and they are not flaky, once I fetched https://github.com/MudBlazor/MudBlazor/actions/runs/8458390892/attempts/2 once I fetched this change I see locally the same picture

[ScarletKuro]

Seems ScrollToTopTest is the only one

[ScarletKuro]

Fixed it in this PR #8506
It's now more spans so it couldn't find the correct element since it was just Find("span").

upd: decided to make separate PR just for this test #8507 and merge it right away, since it might block others and I don't want to make them wait.