Kh/5498 width height event contract#5577
Merged
harder merged 2 commits intoJul 7, 2026
Merged
Conversation
tui-cs#5498 (split off from tui-cs#4522) flagged that Frame/layout-driven size changes bypass the Width/Height change events and skip the TextFormatter reset. Re-evaluated after the related layout/TextFormatter perf work landed (tui-cs#4522, tui-cs#5357, tui-cs#5358/tui-cs#5359, tui-cs#5499, formatter-constraint fixes): - TextFormatter reset is now a non-issue. SetRelativeLayout recomputes the constraints via SetTextFormatterSize () + FinalizeTextFormatterConstraints (), and the Frame setter flows through the same path via Layout (). The "Reset TextFormatter" BUGBUG bullets were stale. - The Width/Height events "gap" is actually the correct design once declarative (Width/Height = Dim) is separated from resolved (Frame = absolute Rectangle). A layout pass changes the Frame, not the Dim (Dim.Fill () stays Dim.Fill ()), and the Frame setter's _width = absolute is bookkeeping, not a user-initiated Width change. FrameChanged already covers resolved-size changes from any cause. Changes (no behavior change): - Replace the two stale BUGBUG blocks in View.Layout.cs (Frame setter + SetRelativeLayout) with accurate design notes. - Document the contract on Width/Height (declarative-assignment only) and on FrameChanged (canonical resolved-size event). - Add ViewLayoutEventTests locking the contract: Width/HeightChanged do NOT fire from the Frame setter or a layout-driven resize; FrameChanged DOES fire in those cases and on declarative assignment after layout. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Documents and tests the intended event-semantics contract between declarative Width/Height (Dim) changes and resolved Frame changes in View, clarifying that layout/Frame-driven size updates should be observed via FrameChanged (Fixes #5498).
Changes:
- Replaces stale
BUGBUGnotes inView.Layout.cswith design notes explaining why internalPos/Dimfields are written directly and whyWidth/Heightchange events are not raised fromFrame/layout paths. - Adds XML documentation to
FrameChanged,Width, andHeightdescribing the declarative-vs-resolved event contract. - Adds unit tests that lock in the contract for direct
Frameassignment and layout-driven resizing.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| Tests/UnitTestsParallelizable/ViewBase/Layout/ViewLayoutEventTests.cs | Adds contract tests ensuring Width/Height Changing/Changed (and hooks) don’t fire for Frame/layout changes, while FrameChanged does. |
| Terminal.Gui/ViewBase/View.Layout.cs | Updates inline comments and XML docs to formally document the Width/Height vs FrameChanged event contract. |
tig
approved these changes
Jul 5, 2026
tig
left a comment
tig
left a comment
Member
There was a problem hiding this comment.
Thank you. This is long overdue and the right fix.
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.
Fixes #5498 — Document the Width/Height vs Frame event contract
Background
#5498 flagged that frame/layout-driven size changes bypass the
Width/HeightChanging/Changedevents and might leaveTextFormatter.ConstrainTo*stale.The issue asked for a decision:
Width/Heightevents from theFrame/layout paths.Dimassignment, and useFrameChangedfor resolved-size changes.This PR takes option 2: document the contract and lock the current behavior with tests.
Findings
The
TextFormatterconcern is no longer a live bug.SetRelativeLayoutnow recomputes formatter constraints throughSetTextFormatterSize ()andFinalizeTextFormatterConstraints (). DirectFrameassignment also flows through that path viaLayout (), so the old// BUGBUG: - Reset TextFormattercomments were stale.The event behavior is intentional once the model is separated into:
Width/HeightasDimFrameas an absoluteRectangleA layout pass can change
Frame.WidthwhileWidthremainsDim.Fill (). RaisingWidthChangedin that case would imply the declarativeDimchanged, which is not true. DirectFrameassignment does overwrite the internalDimfields with absolute values, but that is bookkeeping after theFramehas already changed, not a cancellable declarativeWidth/Heightassignment.Contract:
WidthChanging/WidthChangedandHeightChanging/HeightChangedobserve declarativeDimassignment only.FrameChangedis the event for resolved position/size changes from any cause.Changes
BUGBUGblocks inView.Layout.cswith design notes explaining the direct field writes.Width,Height, andFrameChangeddescribing the event contract.WidthChanged/HeightChangeddo not fire from directFrameassignment.WidthChanging/HeightChangingdo not fire from directFrameassignment.OnWidthChanging/OnWidthChangedandOnHeightChanging/OnHeightChangedare not invoked from directFrameassignment.FrameChangeddoes fire from directFrameassignment, layout-driven resize, and declarative assignment after layout.Notes
FrameChangedfires afterFramehas been updated. It should be used to observe the resolvedFrame, not as a signal that every secondary layout/formatter side effect in the caller path has completed.Testing
dotnet test --project Tests/UnitTestsParallelizable --filter-class "*ViewLayoutEventTests"dotnet build Terminal.Gui/Terminal.Gui.csproj --no-restore -t:Rebuild -v:minimalView.Drawing.csgit diff --check upstream/develop...HEADActual changes are comments/XML docs only; behavior is unchanged.