Skip to content

Conversation

adamint
Copy link
Member

@adamint adamint commented Aug 28, 2025

Description

PaneHeader seems like an appropriate typography for dialog headers, but it renders as an h4, which is too high of a heading number for the importance of the content. h2 is the appropriate level. No change in appearance

Before:
image

After:
image

Related: #10450

Checklist

  • Is this feature complete?
    • Yes. Ready to ship.
    • No. Follow-up changes expected.
  • Are you including unit tests for the changes and scenario tests if relevant?
    • Yes
    • No
  • Did you add public API?
    • Yes
      • If yes, did you have an API Review for it?
        • Yes
        • No
      • Did you add <remarks /> and <code /> elements on your triple slash comments?
        • Yes
        • No
    • No
  • Does the change make any security assumptions or guarantees?
    • Yes
      • If yes, have you done a threat model and had a security review?
        • Yes
        • No
    • No
  • Does the change require an update in our Aspire docs?

@adamint adamint requested review from JamesNK and Copilot August 28, 2025 13:10
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates dialog headers in the Aspire Dashboard to use more semantically appropriate heading levels by changing from Typography.PaneHeader (which renders as h4) to Typography.H2 for dialog titles.

  • Changes dialog headers from h4 to h2 heading level for better semantic hierarchy
  • Affects two dialog components that display titles to users

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
TextVisualizerDialog.razor Updates dialog title typography from PaneHeader to H2
InteractionsInputDialog.razor Updates dialog title typography from PaneHeader to H2

@JamesNK
Copy link
Member

JamesNK commented Aug 29, 2025

This PR changes the dialogs where we customize the header. What about dialogs that use the built in header template? They'll still be <h4>

@adamint
Copy link
Member Author

adamint commented Aug 29, 2025

This PR changes the dialogs where we customize the header. What about dialogs that use the built in header template? They'll still be <h4>

That will require changes in fluentui-blazor that I have to discuss with them about. This is a good start. The only urgent change we need is the ai assistant chat, but since we define custom headers in these two dialogs, we should also change their heading level.

@JamesNK
Copy link
Member

JamesNK commented Sep 10, 2025

Actually, I think there could be a better fix for this. We could update markdown rendering so it doesn't output h2: xoofx/markdig#368

@mitchdenny
Copy link
Member

TheOfficePamBeeslyGIF

Copy link
Member

@JamesNK JamesNK left a comment

Choose a reason for hiding this comment

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

Prefer that we adjust markdown

#11321

@dotnet-policy-service dotnet-policy-service bot added the needs-author-action An issue or pull request that requires more info or actions from the author. label Sep 10, 2025
@adamint
Copy link
Member Author

adamint commented Sep 11, 2025

Prefer that we adjust markdown

#11321

We need to change the header levels. h4 is not appropriate for a modal title and is misleading. The markdown is not the issue! 😄

@dotnet-policy-service dotnet-policy-service bot removed the needs-author-action An issue or pull request that requires more info or actions from the author. label Sep 11, 2025
@adamint adamint requested a review from JamesNK September 11, 2025 00:10
@JamesNK
Copy link
Member

JamesNK commented Sep 11, 2025

The current typo value is Typography.PaneHeader. If FluentUI changes that type to resolve to an h2 then we don't need to make this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants