From 89ffe0d0ad5f185c4aa0d4cb24bbf58ab6fe0eca Mon Sep 17 00:00:00 2001 From: Sheng Chen Date: Thu, 18 Jun 2026 17:25:15 +0800 Subject: [PATCH] fix: keep Preferences dialog stable on Tool Auto Approve page The Tool Auto Approve page stacked four rule sections (~850px) in an uncapped container, so JFace's PreferenceDialog grew the shell toward screen height and never shrank it. Wrap the sections in a height-capped ScrolledComposite whose computeSize bounds the height reported to the dialog's PageLayout, so the shell stays a normal size and the content scrolls within a single page-level scrollbar. Unify the cap as PreferencePageUtils.STANDARD_CONTENT_HEIGHT (520), shared with McpPreferencePage's two stacked groups, so Copilot preference pages settle at a consistent size. Add a generic preference-pages test plan with a dialog-size regression case. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../preference-pages/preference-pages.md | 81 +++++++++++++++++++ .../AutoApprovePreferencePage.java | 34 +++++++- .../ui/preferences/McpPreferencePage.java | 2 +- .../ui/preferences/PreferencePageUtils.java | 9 +++ 4 files changed, 121 insertions(+), 5 deletions(-) create mode 100644 com.microsoft.copilot.eclipse.swtbot.test/test-plans/preference-pages/preference-pages.md diff --git a/com.microsoft.copilot.eclipse.swtbot.test/test-plans/preference-pages/preference-pages.md b/com.microsoft.copilot.eclipse.swtbot.test/test-plans/preference-pages/preference-pages.md new file mode 100644 index 00000000..08555695 --- /dev/null +++ b/com.microsoft.copilot.eclipse.swtbot.test/test-plans/preference-pages/preference-pages.md @@ -0,0 +1,81 @@ +# GitHub Copilot Preference Pages + +## Overview +Covers behavior shared by **all** GitHub Copilot preference pages +(**Preferences → GitHub Copilot → …**), independent of any single feature. +Every Copilot page is hosted in the same JFace `PreferenceDialog`, so concerns +like dialog sizing, layout stability, and navigation are cross-cutting. + +JFace grows the shared dialog to the **tallest** page visited and never shrinks +it again. To keep the dialog a stable size, each Copilot page keeps its content +within a common target height — `PreferencePageUtils.STANDARD_CONTENT_HEIGHT`. +Pages whose natural content is taller (e.g. **Tool Auto Approve**, which stacks +four rule sections) scroll within a height-capped `ScrolledComposite` instead of +ballooning the dialog. The same constant drives `McpPreferencePage`'s two groups, +so the pages settle at a consistent size. + +Entry points exercised: +- **Preferences → GitHub Copilot** — the root category page (only hyperlinks; no + sign-in, always valid). A stable baseline for size comparisons. +- **Preferences → GitHub Copilot → Tool Auto Approve** — the tallest page; the + regression magnet for dialog ballooning. +- Other child pages (MCP, Completions, Chat, …) — for navigation stability. + +--- + +## Prerequisites +- Eclipse IDE with the GitHub Copilot for Eclipse plugin installed and activated. +- No sign-in required: the pages below render independently of language-server + data. + +--- + +## 1. Dialog size stability + +### TC-001: Opening a tall page does not balloon the Preferences dialog + +**Type:** `Regression` +**Priority:** `P1` + +#### Steps +1. Open **Window → Preferences** and select **GitHub Copilot** (root page). Note + the dialog's size. +2. In the tree, select **GitHub Copilot → Tool Auto Approve**. +3. Observe the dialog does **not** grow toward screen height — it stays close to + the root page's size. +4. Confirm all four sections (Terminal, File Operation, MCP, Global) are + reachable by scrolling **within the page** using a single page-level vertical + scrollbar. +5. Navigate to a couple of sibling pages (e.g. **MCP**, **Completions**) and back + to Tool Auto Approve; the dialog size stays stable throughout. +6. Close Preferences. + +#### Expected Result +- The Preferences dialog stays a normal, stable size as the selection moves + between Copilot pages; Tool Auto Approve does not balloon toward screen height. +- Tool Auto Approve content is reachable via the page's own single vertical + scrollbar; no second (dialog-level) scrollbar appears. + +#### Reference measurements (Eclipse 2025-03, macOS) +- Root **GitHub Copilot** page dialog height: **~551px**. +- **Tool Auto Approve** dialog height (fixed by the height cap): **~656px** — + deterministic and screen-independent. +- Pre-fix, Tool Auto Approve ballooned to ~screen height (≈986px or + screen-clamped). + +#### 📸 Key Screenshots +- [ ] Dialog on the root **GitHub Copilot** page. +- [ ] Dialog on **Tool Auto Approve** — only slightly taller, content scrolling + within the page (single page scrollbar). + +--- + +## Notes +- Page height is **data-independent** (table/tree height hints are fixed), so MCP + servers loading asynchronously does not change the page height. +- The height cap is unified via `PreferencePageUtils.STANDARD_CONTENT_HEIGHT`, + shared by `AutoApprovePreferencePage` (scroller cap) and `McpPreferencePage` + (two stacked groups). Keep the pages within this height when adding content. +- Use the root **GitHub Copilot** page as the size baseline — it is always valid + and needs no sign-in, whereas the MCP page can enter an invalid state in an + unauthenticated sandbox and pop a JFace validation dialog. diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/AutoApprovePreferencePage.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/AutoApprovePreferencePage.java index b4b8b3f9..fb0be2c0 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/AutoApprovePreferencePage.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/AutoApprovePreferencePage.java @@ -6,6 +6,8 @@ import org.eclipse.jface.preference.IPreferenceStore; import org.eclipse.jface.preference.PreferencePage; import org.eclipse.swt.SWT; +import org.eclipse.swt.custom.ScrolledComposite; +import org.eclipse.swt.graphics.Point; import org.eclipse.swt.layout.GridData; import org.eclipse.swt.layout.GridLayout; import org.eclipse.swt.widgets.Composite; @@ -51,12 +53,28 @@ protected Control createContents(Composite parent) { Messages.preferences_page_auto_approve_disabled_by_organization); } - Composite root = new Composite(parent, SWT.NONE); + // Wrap the sections in a height-capped ScrolledComposite. The override on + // computeSize bounds the height reported to the dialog's PageLayout so the + // Preferences shell stays stable; the real (taller) content scrolls here. + // This scroller is also the parent the section tables forward wheel events + // to (see forwardVerticalMouseWheelToParentScrollerAtBoundary). + ScrolledComposite scrolled = new ScrolledComposite(parent, SWT.V_SCROLL) { + @Override + public Point computeSize(int widthHint, int heightHint, boolean changed) { + Point size = super.computeSize(widthHint, heightHint, changed); + size.y = Math.min(size.y, PreferencePageUtils.STANDARD_CONTENT_HEIGHT); + return size; + } + }; + scrolled.setExpandHorizontal(true); + scrolled.setExpandVertical(true); + scrolled.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, true)); + + Composite root = new Composite(scrolled, SWT.NONE); GridLayout layout = new GridLayout(1, false); layout.marginWidth = 0; layout.marginHeight = 0; root.setLayout(layout); - root.setLayoutData(new GridData(SWT.FILL, SWT.TOP, true, false)); IPreferenceStore store = getPreferenceStore(); terminalSection = new TerminalAutoApproveSection(root, SWT.NONE); @@ -72,9 +90,17 @@ protected Control createContents(Composite parent) { globalSection = new GlobalAutoApproveSection(root, SWT.NONE); globalSection.loadFromPreferences(store); - root.addDisposeListener(e -> unbindMcpConfigService()); + scrolled.setContent(root); + scrolled.setMinSize(root.computeSize(SWT.DEFAULT, SWT.DEFAULT)); + // Track width so wrapping labels reflow and only vertical scrolling occurs. + scrolled.addListener(SWT.Resize, e -> { + int width = scrolled.getClientArea().width; + scrolled.setMinSize(root.computeSize(width, SWT.DEFAULT)); + }); + + scrolled.addDisposeListener(e -> unbindMcpConfigService()); - return root; + return scrolled; } @Override diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/McpPreferencePage.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/McpPreferencePage.java index c58c3947..c41975d0 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/McpPreferencePage.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/McpPreferencePage.java @@ -83,7 +83,7 @@ public class McpPreferencePage extends FieldEditorPreferencePage implements IWorkbenchPreferencePage { public static final String ID = "com.microsoft.copilot.eclipse.ui.preferences.McpPreferencePage"; - private static final int GROUP_HEIGHT_HINT = 260; + private static final int GROUP_HEIGHT_HINT = PreferencePageUtils.STANDARD_CONTENT_HEIGHT / 2; private static final Gson GSON = new Gson(); private Group toolsGroup; diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/PreferencePageUtils.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/PreferencePageUtils.java index ab676ebf..13ed7ea9 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/PreferencePageUtils.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/preferences/PreferencePageUtils.java @@ -25,6 +25,15 @@ */ public final class PreferencePageUtils { + /** + * Target content height, in pixels, for Copilot preference pages. JFace grows the shared Preferences dialog to + * the tallest page's preferred height and never shrinks it, so each page keeps its scrollable content within + * this height to hold the dialog at a stable size while the user navigates. Pages enforce it differently: + * {@code McpPreferencePage} divides it across two stacked groups; {@code AutoApprovePreferencePage} caps its + * {@code ScrolledComposite} at this height. + */ + public static final int STANDARD_CONTENT_HEIGHT = 520; + // Private constructor to prevent instantiation private PreferencePageUtils() { }