Skip to content

Moving connection dialog to dialog shell.#21644

Merged
aasimkhan30 merged 5 commits intomainfrom
aasim/fix/dialogShell2
Mar 18, 2026
Merged

Moving connection dialog to dialog shell.#21644
aasimkhan30 merged 5 commits intomainfrom
aasim/fix/dialogShell2

Conversation

@aasimkhan30
Copy link
Contributor

@aasimkhan30 aasimkhan30 commented Mar 17, 2026

Description

image

Code Changes Checklist

  • New or updated unit tests added
  • All existing tests pass (npm run test)
  • Code follows contributing guidelines
  • Telemetry/logging updated if relevant
  • No regressions or UX breakage

Reviewers: Please read our reviewer guidelines

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR migrates the Connection Dialog webview to the shared DialogPageShell layout so it matches the newer standardized dialog UI patterns across the extension.

Changes:

  • Wrap the Connection Dialog in DialogPageShell, moving the primary “Connect” action and “Advanced settings” action into the shell footer.
  • Centralize the AdvancedOptionsDrawer and remove per-page footer trays/buttons from the parameter/Azure/Fabric connection pages.
  • Apply minor spacing tweaks to the MRU connections list UI and remove the now-redundant ConnectionHeader component.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
extensions/mssql/src/reactviews/pages/ConnectionDialog/connectionPage.tsx Adopts DialogPageShell with footer actions and hosts AdvancedOptionsDrawer at the page level.
extensions/mssql/src/reactviews/pages/ConnectionDialog/connectionPageContainer.tsx Refactors form structure (adds form id) so the connect action can submit from the shell footer; reorganizes message/dialog rendering.
extensions/mssql/src/reactviews/pages/ConnectionDialog/components/connectButton.component.tsx Adds optional form prop to support submitting a form from outside its DOM subtree.
extensions/mssql/src/reactviews/pages/ConnectionDialog/azureBrowsePage.tsx Removes the per-page advanced drawer + connect footer tray (now handled by the shell).
extensions/mssql/src/reactviews/pages/ConnectionDialog/fabricBrowsePage.tsx Removes the per-page advanced drawer + connect footer tray (now handled by the shell).
extensions/mssql/src/reactviews/pages/ConnectionDialog/connectionFormPage.tsx Removes the per-page advanced drawer + connect footer tray (now handled by the shell).
extensions/mssql/src/reactviews/pages/ConnectionDialog/connectionsListContainer.tsx Tweaks styling/spacing and sets connection cards to a smaller visual density.
extensions/mssql/src/reactviews/common/dialogPageShell.tsx Updates header sizing/styling and footer padding as part of the dialog-shell adoption.
extensions/mssql/src/reactviews/pages/ConnectionDialog/components/connectionHeader.component.tsx Deletes the old header component in favor of the dialog shell header.

borderTop: "1px solid var(--vscode-editorGroup-border)",
backgroundColor: "var(--vscode-editorWidget-background, var(--vscode-editor-background))",
padding: "12px 0",
padding: "12px 16px",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, intentional as we don't want footer buttons to stick to sides and also be a little broader than the content

value: option.value,
});
<div className={formStyles.formComponentDiv}>
<Field label="Input type" orientation="horizontal">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aasimkhan30 can you fix this while you're poking this file?

@github-actions
Copy link

github-actions bot commented Mar 17, 2026

PR Changes

Category Target Branch PR Branch Difference
vscode-mssql VSIX 6441 KB 6438 KB ⚪ -3 KB ( 0% )
sql-database-projects VSIX 7062 KB 7062 KB ⚪ 0 KB ( 0% )
data-workspace VSIX 535 KB 535 KB ⚪ 0 KB ( 0% )

@codecov-commenter
Copy link

codecov-commenter commented Mar 17, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.95%. Comparing base (fecd36c) to head (bdb9548).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main   #21644   +/-   ##
=======================================
  Coverage   72.95%   72.95%           
=======================================
  Files         331      331           
  Lines       99209    99209           
  Branches     5582     5582           
=======================================
  Hits        72374    72374           
  Misses      26835    26835           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

lewis-sanchez
lewis-sanchez previously approved these changes Mar 17, 2026
Benjin
Benjin previously approved these changes Mar 17, 2026
Copilot AI review requested due to automatic review settings March 17, 2026 22:55
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Moves the Connection Dialog UI onto the shared DialogPageShell layout to standardize header/content/footer behavior and consolidate the Connect/Advanced Settings actions into the shell footer.

Changes:

  • Wrap the main connection form in DialogPageShell and relocate Connect + Advanced Settings controls into the dialog footer.
  • Remove per-page nav trays (Connect/Advanced) from the Parameters/Azure/Fabric pages and delete the legacy ConnectionHeader component.
  • Update shared shell styling (header/footer layout) and tweak connection list spacing/card sizing.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
extensions/mssql/src/reactviews/pages/ConnectionDialog/connectionPage.tsx Hosts the connection dialog content in DialogPageShell and adds footer actions + advanced drawer state.
extensions/mssql/src/reactviews/pages/ConnectionDialog/connectionPageContainer.tsx Adds a stable form id for external submit and adjusts padding/layout for shell-based rendering.
extensions/mssql/src/reactviews/pages/ConnectionDialog/components/connectButton.component.tsx Adds optional form support to enable footer-based form submission with the existing split-button UX.
extensions/mssql/src/reactviews/pages/ConnectionDialog/connectionFormPage.tsx Removes inline nav tray (Connect/Advanced) now handled by the shell footer.
extensions/mssql/src/reactviews/pages/ConnectionDialog/azureBrowsePage.tsx Removes inline Connect/Advanced controls to rely on shell footer actions.
extensions/mssql/src/reactviews/pages/ConnectionDialog/fabricBrowsePage.tsx Removes inline Connect/Advanced controls to rely on shell footer actions.
extensions/mssql/src/reactviews/pages/ConnectionDialog/connectionsListContainer.tsx Adjusts spacing and card sizing for better alignment with the updated dialog layout.
extensions/mssql/src/reactviews/common/dialogPageShell.tsx Updates shell header/footer styling and increases header icon sizing.
extensions/mssql/src/reactviews/pages/ConnectionDialog/components/connectionHeader.component.tsx Removes legacy header component superseded by DialogPageShell.

Comment on lines +88 to +90
<form id="connectionForm" onSubmit={handleConnect}>
<button type="submit" style={{ display: "none" }} aria-hidden="true" tabIndex={-1} />
<ConnectionHeader />

<div className={formStyles.formDiv} style={{ overflow: "auto" }}>
<div className={styles.formContent}>
@aasimkhan30 aasimkhan30 merged commit e50a268 into main Mar 18, 2026
7 checks passed
@aasimkhan30 aasimkhan30 deleted the aasim/fix/dialogShell2 branch March 18, 2026 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants