fix: consolidate control validation into layoutNow#340
Merged
Conversation
requestLayout and flushPendingLayoutRequests each filtered out null/ disposed controls via a shared validControls helper before delegating to layoutNow, duplicating the same defensive check at every call site. layoutNow now performs its own null/disposed filtering right where it dereferences the controls (getShell(), shell.layout(...)), so the invariant is enforced in exactly one place instead of trusted by two separate callers. requestLayout becomes a pure dispatcher: it no longer pre-filters, it just forwards the raw controls to pendingLayoutRequests or layoutNow. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors Chat view layout triggering to avoid expensive relayout work while the Chat view is hidden (e.g., stacked behind another part), addressing UI freezes described in issue #335. It centralizes control validation inside the layout executor and introduces a deferred “replay” mechanism when the view becomes visible again.
Changes:
- Replaces the previous full-view layout call path with a targeted
requestLayout(Control...)API that defers requests while the view is not visible. - Adds a
pendingLayoutRequestsqueue and flushes it onpartVisibleto replay suppressed layout requests. - Updates
ReferencedFile#setFile(...)to request layout for the specific child controls that may change rather than triggering a full layout.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ReferencedFile.java | Switches from chatView.layout(true, true) to targeted chatView.requestLayout(...) calls to avoid full relayout and stale cached sizes. |
| com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatView.java | Introduces deferred targeted layout (requestLayout, pendingLayoutRequests, flushPendingLayoutRequests, layoutNow) and flush-on-visible behavior. |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
The prior "Potential fix for pull request finding" commit (04e7682) badly spliced a null-check into requestLayout, leaving a dangling duplicate `layoutNow(changed); }` outside the method body and dedenting the whole method. This left invalid Java that failed the Tycho/ECJ compile in CI. Restores correct syntax/indentation while keeping the null-guard around Collections.addAll(pendingLayoutRequests, changed), which is the same defensive check layoutNow already applies to its own `controls` parameter. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.
requestLayout and flushPendingLayoutRequests each filtered out null/ disposed controls via a shared validControls helper before delegating to layoutNow, duplicating the same defensive check at every call site.
layoutNow now performs its own null/disposed filtering right where it dereferences the controls (getShell(), shell.layout(...)), so the invariant is enforced in exactly one place instead of trusted by two separate callers. requestLayout becomes a pure dispatcher: it no longer pre-filters, it just forwards the raw controls to pendingLayoutRequests or layoutNow.
fix #335