Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Use h1 as first heading in dialogs #12244

Closed
wants to merge 6 commits into from
Closed

Use h1 as first heading in dialogs #12244

wants to merge 6 commits into from

Conversation

dbkr
Copy link
Member

@dbkr dbkr commented Feb 13, 2024

The dialog should be modal and therefore the only content active on the screen, so the title of the dialog should be the top level.

Checklist

  • Tests written for new code (and old code if feasible)
  • Linter and other CI checks pass
  • Sign-off given on the changes (see CONTRIBUTING.md)

Here's what your changelog entry will look like:

🐛 Bug Fixes

  • Use h1 as first heading in dialogs (#12244).

The dialog should be modal and therefore the only content active
on the screen, so the title of the dialog should be the top level.
@dbkr dbkr added the T-Defect Bugs, crashes, hangs, vulnerabilities, or other reported problems label Feb 13, 2024
They were almost identical apart from the letter spacing
@@ -163,6 +163,7 @@ b {
font-weight: bold;
}

h1,
Copy link
Member

@robintown robintown Feb 13, 2024

Choose a reason for hiding this comment

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

Because letter-spacing is included in this declaration, I believe this is why things like the header on the onboarding page have changed their letter spacing. Probably the correct solution is to make sure .mx_Heading_h1 gets its own letter-spacing rule, to match the "heading XL semibold" font

Screenshot 2024-02-13 at 10-12-02 Playwright Test Report

@@ -33,6 +33,7 @@ limitations under the License.
.mx_Heading_h3 {
font: var(--cpd-font-heading-md-semibold);
font-weight: var(--cpd-font-weight-semibold);
letter-spacing: var(--cpd-font-letter-spacing-heading-lg);
Copy link
Member

Choose a reason for hiding this comment

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

Why -lg and not -md?

Copy link
Member Author

Choose a reason for hiding this comment

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

So this was the letter spacing used for h2s so it's what makes them match what they were before. I'm also wondering why they're styled as h3s though - this might be what makes it odd.

@dbkr
Copy link
Member Author

dbkr commented Feb 14, 2024

This merge has gone terribly so I'm going to try again in a new branch.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T-Defect Bugs, crashes, hangs, vulnerabilities, or other reported problems
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants