Skip to content

Remove app bar item horizontal padding#7221

Merged
JamesNK merged 3 commits intomicrosoft:mainfrom
adamint:dev/adamint/fix-cutoff-appbar-items
Jan 28, 2025
Merged

Remove app bar item horizontal padding#7221
JamesNK merged 3 commits intomicrosoft:mainfrom
adamint:dev/adamint/fix-cutoff-appbar-items

Conversation

@adamint
Copy link
Copy Markdown
Member

@adamint adamint commented Jan 23, 2025

Description

Nav bar items were being cut off due to a default app bar item width. Since there's a wide range of text lengths for these strings based on language, I un-set the width to allow the app bar width to flex to fit content.

Before
english:
image

spanish:
image

After
english (slightly narrower):
image

spanish (wider):
image

There is one extreme (Russian), I'm not sure what to do with this. Maybe setting a max width? @JamesNK
image

Fixes #6935

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 drewnoakes January 23, 2025 18:52
@JamesNK
Copy link
Copy Markdown
Member

JamesNK commented Jan 24, 2025

I don't like this. I can't think of other UIs that have this design where the app bar changes size. I looked at what teams does when it is set to spanish and the app bar size is the same but the text is displayed up to the edges.

Instead to fix this we can remove the left/right padding on the items in the app bar. That allows all content to be displayed:

image

That won't work for every language - I see your Russian example won't fit - but I think we have to draw a line somewhere. This approach gives more room, but keeps a fixed size.

Maybe we should ask translation folks whether there are shorter words that can be used for Russian?

Copy link
Copy Markdown
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.

See comment

@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 Jan 24, 2025
@adamint
Copy link
Copy Markdown
Member Author

adamint commented Jan 27, 2025

I don't like this. I can't think of other UIs that have this design where the app bar changes size. I looked at what teams does when it is set to spanish and the app bar size is the same but the text is displayed up to the edges.

Instead to fix this we can remove the left/right padding on the items in the app bar. That allows all content to be displayed:

image

That won't work for every language - I see your Russian example won't fit - but I think we have to draw a line somewhere. This approach gives more room, but keeps a fixed size.

Maybe we should ask translation folks whether there are shorter words that can be used for Russian?

Sounds good with me. I do agree with

I think we have to draw a line somewhere.

especially considering it's only one item.

@adamint adamint requested a review from JamesNK January 27, 2025 22:57
@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 Jan 27, 2025
@adamint adamint changed the title Allow app bar to flex width based on content Remove app bar item horizontal padding Jan 27, 2025
Comment thread src/Aspire.Dashboard/Components/Layout/MainLayout.razor.css Outdated
@JamesNK JamesNK enabled auto-merge (squash) January 28, 2025 00:32
@JamesNK JamesNK merged commit 2f41b28 into microsoft:main Jan 28, 2025
@github-actions github-actions Bot locked and limited conversation to collaborators Feb 27, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[WebToolsE2E][Aspire] The displayed string for Structured is not fully displayed for some localization.

2 participants