Fix button auto-size text measurement#7
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes a retained-UI layout inconsistency where Button auto-size width was computed via a character-count heuristic while rendering centered/drew text using measured glyph widths, which could cause text to overflow a “correctly rendered but incorrectly measured” button.
Changes:
- Update
Button.MeasureCoreto use the retained UI root’s active text measurement path when available (falling back to the prior heuristic otherwise). - Add a regression test that proves measured wide text (“WWW”) expands an auto-sized button’s desired/bounds width.
- Document the measurement/render consistency lesson in
Lessons.md.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/ModernOverlay.Tests/OverlayUiButtonControlTests.cs | Adds a Windows integration regression test and extends the test draw sink to simulate wide measured text. |
| src/ModernOverlay.UI/Controls.cs | Updates Button auto-size measurement to prefer active measured text widths when a layout measurement context exists. |
| Lessons.md | Records the retained-UI measurement/render consistency guideline for text-rendering controls. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
555
to
559
| float fontSize = Root?.ThemeResources.Theme.FontSize ?? UiTheme.Default.FontSize; | ||
| float width = MathF.Min(availableSize.Width, Text.Length * fontSize * 0.6f + Padding.Horizontal); | ||
| float height = fontSize * 1.35f + Padding.Vertical; | ||
| SizeF textSize = MeasureButtonText(Text, fontSize); | ||
| float width = MathF.Min(availableSize.Width, textSize.Width + Padding.Horizontal); | ||
| float height = textSize.Height * 1.35f + Padding.Vertical; | ||
| return new SizeF(MathF.Max(MinWidth, width), MathF.Max(MinHeight, height)); |
Comment on lines
+173
to
+176
| ui.Render(new DrawContext(new RecordingDrawCommandSink())); | ||
|
|
||
| Assert.AreEqual(140f, button.DesiredSize.Width, 0.001f); | ||
| Assert.AreEqual(140f, button.Bounds.Width, 0.001f); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Buttonauto-size measurement to use the active UI text measurement path instead of a character-count heuristic when a render measurement context is available.Lessons.md.Root cause
Button.RenderCorecentered and drew text using measured glyph width, butButton.MeasureCoresized text-only buttons withText.Length * fontSize * 0.6f. With proportional or otherwise wide measured text, an auto-sized button could report a much narrower desired width than the text it later rendered.Validation
dotnet test tests\ModernOverlay.Tests\ModernOverlay.Tests.csproj --configuration Debug --filter "FullyQualifiedName~ButtonAutoSizeUsesMeasuredTextWidth"dotnet test tests\ModernOverlay.Tests\ModernOverlay.Tests.csproj --configuration Debug --filter "FullyQualifiedName~OverlayUiButtonControlTests"dotnet test tests\ModernOverlay.Tests\ModernOverlay.Tests.csproj --configuration Debug --logger trxdotnet test tests\ModernOverlay.Tests\ModernOverlay.Tests.csproj --configuration Release --filter "FullyQualifiedName~ButtonAutoSizeUsesMeasuredTextWidth"git diff --check