Release v2.4.3#5447
Merged
Merged
Conversation
…n-out Capture NeedsDraw at the top of View.Draw() before DoDrawAdornments mutates NeedsDrawRect, and gate DoClearViewport / DoDrawText / DoDrawContent on the captured value. When a parent enters Draw() only because SubViewNeedsDraw is true, its viewport is no longer cleared and its own text/content is no longer redrawn. Adornments, subviews, and line canvas still render correctly. Refs #5358 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The escalation remains necessary for DoRenderLineCanvas and ClearNeedsDraw, but it no longer drives parent self-content redraws — those are now gated on the needsDrawSelf snapshot captured before this escalation. Updated the inline comment to reflect the narrowed purpose. Refs #5358 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ing draw Remove the redundant SetNeedsDraw() call in the base AdornmentView's OnClearingViewport. The thickness has just been drawn; calling SetNeedsDraw mid-pass cascades to the parent's SubViewNeedsDraw and adds churn that competes with the needsDrawSelf gate introduced for #5358. MarginView already overrides without this call. BorderView and PaddingView, which inherit the base implementation, also benefit. Also refresh the stale class doc comment — Border and Padding now extend AdornmentView too. Refs #5358 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Five tests covering the needsDrawSelf gate: - ChildOnlyDirty: parent does not clear/redraw self - ParentDirectlyDirty: full redraw path still works (regression guard) - TransparentParent: transparent early-return composes with the gate - ParentWithBorder: parent skips clear/text even with an adornment present - IOutput layer: child's render still reaches IOutput after the fix Refs #5358 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
DrawComplete fires unconditionally for clip-exclusion bookkeeping even when NeedsDraw is false, so it is the wrong signal for measuring whether a view actually did draw work. Add a DrawingText counter to both the diagnostic and integration tests — DrawingText is gated on NeedsDraw inside DoDrawText and is not intercepted by Code (which does intercept ClearedViewport / DrawingContent via OnClearingViewport / OnDrawingContent returning true). Use DrawingText to verify per-Code inactive-tab behavior, and use tabsCounts.ClearedViewport to verify the parent's own behavior. TabsFanOutDiagnosticTests (synthetic Layout/Draw path) now asserts: - inactive tabs have DrawingText == 0 - Tabs container has ClearedViewport == 0 - text-draw fan-out ratio == 1.0 (parity with single-Code baseline) TabsFanOutIntegrationTests (end-to-end via main loop) keeps the existing "documents remaining fan-out" assertion: a separate cascade source remains in ApplicationImpl.LayoutAndDraw (force=true when neededLayout, calling SetNeedsDraw on the top runnable which cascades to overlapping subviews). Removing that force=true uncovers stale-content bugs in the shrink/move path (already covered by ShadowTests / BorderViewTests) and is out of scope for #5358. Layout fan-out is fully eliminated by PR #5373. Refs #5358 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous union math at View.NeedsDraw.cs lines 87-92 was buggy: int x = Math.Min (Viewport.X, viewPortRelativeRegion.X); int y = Math.Min (Viewport.Y, viewPortRelativeRegion.Y); int w = Math.Max (Viewport.Width, viewPortRelativeRegion.Width); int h = Math.Max (Viewport.Height, viewPortRelativeRegion.Height); It unioned against Viewport (not against the existing NeedsDrawRect), and used max-of-Width / max-of-Height rather than Rectangle.Union. The result was that NeedsDrawRect was effectively clamped to the current Viewport size on every call — accumulated partial regions were silently widened to ~viewport-wide, making NeedsDrawRect useless for narrowing draw work. Replace with Rectangle.Union (NeedsDrawRect, viewPortRelativeRegion) so NeedsDrawRect accurately tracks the accumulated dirty area. This is required for issue #5358's region-aware ClearViewport. Update NeedsDrawTests to reflect the new accumulating semantics: - NeedsDrawRect_Is_Viewport_Relative: shrink scenarios now keep the larger accumulated area (Union of prior larger frame and current smaller one) - SetNeedsDraw_MultipleRectangles_Expands: union of two non-overlapping rects is the bounding rect, not the full viewport Refs #5358 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When NeedsDrawRect is set and genuinely smaller than the full viewport (i.e. a real partial-region invalidation, not SetNeedsDraw()-no-arg which sets the full Viewport), narrow the clear and the trailing SetNeedsDraw cascade to just that region. Otherwise preserve the existing full-clear contract used by Code.OnClearingViewport, MarkdownCodeBlock, and direct test callers (e.g. ClearViewport_FillsViewportArea, ClearViewport_SetsNeedsDraw). This is the second half of the infrastructure for issue #5358's narrowed draw work: combined with the SuperView invalidation on Frame change added in the next commit, geometry changes will only clear and cascade to the affected region instead of forcing the whole viewport and all overlapping peers to redraw. Refs #5358 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When a view's Frame shrinks or moves, the SuperView's old-frame area is now uncovered and must be cleared on the next draw — otherwise stale content remains (e.g. an old shadow after a view shrinks). Add a SuperView.SetNeedsDraw (Rectangle.Union(originalFrame, Frame)) call in View.Layout(Size) when frameChanged. Combined with the region-aware ClearViewport from the previous commit, this clears only the union area and cascades NeedsDraw only to overlapping subviews intersecting that union — not the entire viewport. Note: this only catches Frame changes. Adornment thickness changes (e.g. a Margin shrinking when a shadow is removed) don't change Frame, so this fix doesn't help them. A future change can extend this by tracking adornment thickness deltas during Layout — at which point ApplicationImpl.LayoutAndDraw can drop force=true on neededLayout and the integration-level draw fan-out in TabsFanOutIntegrationTests can be flipped to assert == 0. Refs #5358 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This is the cascade source that keeps TabsFanOutIntegrationTests bandaged. Removing force=true when only neededLayout (not forceRedraw) requires tracking adornment thickness changes so the SuperView can be invalidated precisely. Until that lands, ShadowTests and BorderViewTests rely on force=true to clear stale content after adornment thickness rebalances that don't change Frame. Refs #5358 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…tests SetFrame is the direct-assignment path for view.Frame = ... (used by code that sets Frame outright). It bypasses Layout()'s frameChanged detection because SetFrame updates _frame before Layout runs. Add the same SuperView.SetNeedsDraw (Union(oldFrame, newFrame)) call in SetFrame so direct Frame assignments also trigger the region-aware uncovered-area clearing. Add RegionAwareClearViewportTests with 5 focused tests: - DirectCaller_EmptyNeedsDrawRect_ClearsFullViewport — backward-compat contract - PartialNeedsDrawRect_ClearsOnlyDirtyRegion — narrowing fires when partial - FullViewportNeedsDrawRect_ClearsFullViewport — SetNeedsDraw() no-arg path - FrameShrink_InvalidatesSuperViewWithUnionOfOldAndNewFrames — Step 3 catches it - Scroll_DoesNotInvalidateSuperView — pure scroll does not fan out Refs #5358 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Build Terminal.Gui.csproj in addition to Terminal.Gui.Interop.Spectre so that 'dotnet pack --no-build' succeeds for both packages - Fix README trigger docs to match actual workflow (develop + tags, not main) - Correct 'Additional actions' section to reflect tag-based triggers Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Back-merge v2.4.2 from main into develop
…es-memory-optimizations
…serve transparent cache CoPilot code review surfaced three correctness issues with the region-aware ClearViewport from the prior commit: 1. Coordinate-space inconsistency. SetNeedsDraw(Rectangle) cascades to subviews using frame-local coordinates (subtracts subview.Frame.X/Y), while the no-arg variant passes content-coord Viewport. For an unscrolled view those coincide; for a scrolled view they disagree, and my narrowing math (subtracting Viewport.X/Y) would shift the clear off-screen on a scrolled subview. 2. Code/MarkdownCodeBlock full-clear contract. Both call public ClearViewport() from their OnClearingViewport overrides expecting a full fill of the code-block background. With NeedsDrawRect set partial by the cascade, the narrowed clear left stale background in the un-cleared part of the viewport (visible when content has fewer lines than the viewport, etc.). Fix: revert public ClearViewport() to always full-clear. Move narrowing logic into a private CanNarrowClearToNeedsDrawRect helper invoked only from DoClearViewport (the framework's draw-pipeline entry point), guarded by Viewport.Location == Point.Empty so the scrolled-cascade ambiguity is sidestepped entirely. The Item 1 normalization (frame-local vs content-coord NeedsDrawRect) is a deeper refactor deferred as a follow-up. 3. TransparentMouse hit-region wipe. _localDrawContext was recreated unconditionally before the needsDrawSelf gate, but on child-only passes DoDrawText/DoDrawContent are skipped, so _localDrawContext stayed empty. DoDrawComplete then wrote that empty region into CachedDrawnRegion, breaking mouse hit-testing for transparent views until the next full self-redraw. Fix: only recreate _localDrawContext when needsDrawSelf is true. On child-only passes the prior context is preserved, so DoDrawComplete keeps the previously- cached drawn region. SetNeedsDraw nulls CachedDrawnRegion when the view is genuinely invalidated, so stale cache values are bounded by view lifecycle. Updated tests: - RegionAwareClearViewportTests: switched from testing the public ClearViewport API to testing the framework's DoClearViewport via Draw(). Added a FrameworkDraw_ScrolledView_FallsBackToFullClear test that locks in the scrolled-view guard from fix 1. - SubViewOnlyRedrawTests: added TransparentMouseParent_ChildOnlyDirty_ PreservesCachedDrawnRegion to lock in fix 3. Refs #5358 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CoPilot review item 4: SetFrame() already calls SuperView.SetNeedsDraw(union) for every Frame mutation (both direct view.Frame=... and layout-driven paths go through SetFrame). The matching block in Layout(Size) repeated the union and the cascade work on the hot path this PR is trying to trim. The duplicate was idempotent (Rectangle.Union of identical rects produces the same rect), but the cascade-to-overlapping-subviews work ran twice. Refs #5358 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Add tests verifying that both MovingNext cancellation and StepChanging cancellation prevent the accept event from bubbling to the wizard dialog. The StepChanging test exposed a bug: when GoNext() returned false, e.Handled was also false, allowing accept to bubble up and close the wizard. Fix: always set e.Handled = true in NextFinishBtnOnAccepting before calling GoNext(), since the handler has committed to processing the accept regardless of whether navigation succeeds. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Moved Next/Finish navigation logic to Wizard.OnAccepting, centralizing step transitions and event raising. Updated Back button to use a dedicated handler. Switched help text rendering to Markdown with scrollbars. Improved design-time demo and clarified event documentation. Tests updated for new navigation flow and naming consistency.
Refactor object initializations in Wizards.cs and WizardStep.cs to use single-line initializers for improved readability and style consistency. Rename BackButton event handler in Wizard.cs and update IDesignable.GetDemoKeyStrokes to return a non-nullable string. Reformat HelpText in WizardStep.cs. In WizardTests.cs, move and deduplicate Copilot-generated tests for Enter key navigation and event cancellation.
1. Enter_In_TextField_On_Last_Step_Requests_Stop: OnAccepting returns false on last step, bypassing Dialog<TResult>.OnAccepting which calls RequestStop(). Wizard doesn't close when Enter is pressed in content view on last step. 2. MovingBack_Cancel_Does_Not_Bubble_Accept: BackBtnOnAccepting doesn't set e.Handled when MovingBack is cancelled, so accept bubbles to the dialog. Both tests intentionally fail to document the bugs before fixing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…utton Fixes #5430 - Wizard default accept routing from step content
Fixes #5438 - Update CheckState default glyphs
Add !Visible guard to InvokeCommandsBoundToHotKey matching the existing !Enabled guard. This prevents hotkey processing and recursion into subview trees for invisible views, protecting against custom HotKey handlers that don't call DefaultHotKeyHandler. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Added CheckboxMode to ITreeView interface for editor access - Added CheckboxMode toggle checkbox to TreeViewEditor settings panel Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Restructure OnActivated to handle mouse clicks before checking SelectedObject. Previously, if SelectedObject was null, the method returned early without processing mouse events (checkbox toggle, expand/collapse). Now mouse clicks use HitTest directly and only the keyboard activation path checks SelectedObject. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove redundant MouseBindings.Add(LeftButtonClicked, Command.Activate) from TreeView constructor. The base View.DefaultMouseBindings already maps LeftButtonReleased to Command.Activate. Having both caused OnActivated to fire twice per click (once for Released, once for synthesized Clicked), toggling state on then immediately off. Also rewrote tests to use the full mouse pipeline (app.InjectSequence with InputInjectionExtensions.LeftButtonClick) instead of calling NewMouseEvent directly, ensuring tests exercise the same code path as the real application. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Restore GetEffectiveCheckState to derive parent state from children - Add SetCheckedRecursive to propagate check state to all descendants - Parent shows indeterminate (None) when children have mixed states - Toggling parent checks/unchecks all descendants - Update tests to verify tri-state behavior and propagation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add Checkbox Mode section to treeview.md covering tri-state behavior, interaction, programmatic API, and events - Update keyboard/mouse tables to reflect checkbox interactions - Add note to tableview.md Tree Tables section clarifying that TreeTableSource does not render CheckboxMode glyphs — users should use CheckBoxTableSourceWrapper for tree-table checkboxes Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- GetEffectiveCheckState: only derive from expanded/known children, not from lazy TreeBuilder.GetChildren (avoids materializing expensive subtrees during paint) - SetCheckedRecursive: add visited HashSet to prevent infinite recursion on cyclic tree builders - SetCheckedCore: report effective (derived) OldValue in CheckedChanged event, not raw dictionary value - Branch.Draw: account for checkbox cells in scrolled indexOfModelText calculation - Fix doc example: IEnumerable<T> not IReadOnlyCollection<T> - Add tests for cyclic tree builder protection Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- ToggleChecked now correctly handles all cases:
- If effective state is not UnChecked, toggle to UnChecked (propagates to children)
- If effective state is UnChecked but descendants have explicit checked state
(collapsed parent scenario), also toggle to UnChecked
- Only toggle to Checked when fully unchecked with no checked descendants
- Add HasCheckedDescendantsInState helper for collapsed-parent detection
- Add comprehensive mouse-click test that clicks children individually then
unchecks parent (mimics exact UICatalog user scenario)
- Revert Glyphs.cs emoji changes (PR #5439 handles glyph updates separately)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
GetEffectiveCheckState previously required IsExpanded:true to derive state from children. But ChildBranches are preserved in memory after collapse, so the parent can still derive its tri-state correctly without calling TreeBuilder.GetChildren (which was the original perf concern). Fix: Remove the IsExpanded guard from GetEffectiveCheckState so collapsed parents correctly show indeterminate/checked based on their known children. Tests added (all verified to FAIL without fix, PASS with fix): - Collapsed_Parent_Shows_Indeterminate_When_One_Child_Checked - Collapsed_Parent_Shows_Checked_When_All_Children_Checked - Collapsed_Parent_Shows_UnChecked_When_No_Children_Checked Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…de-for-treeview Add built-in TreeView checkbox mode
Fixes #5444 - Remove Dialog.SetStyle; set scheme and arrangement in constructor
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6c9df2c2df
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
This was referenced Jun 1, 2026
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.
Release v2.4.3
Version:
2.4.3NuGet Package:
Terminal.Gui 2.4.3What happens when this PR is merged
v2.4.3main→developwill be openedChecklist
2.4.3