Redesign query result summary and settings#21467
Open
aasimkhan30 wants to merge 22 commits intomainfrom
Open
Conversation
added 10 commits
February 25, 2026 16:53
…ed performance and clarity
… better visibility
…ummary # Conflicts: # extensions/mssql/package.json # extensions/mssql/src/controllers/mainController.ts # extensions/mssql/src/queryResult/queryResultWebViewController.ts # localization/xliff/vscode-mssql.xlf
aasimkhan30
requested review from
Benjin,
allancascante,
caohai,
kburtram,
laurenastrid1,
lewis-sanchez,
manujoseph85 and
ssreerama
as code owners
aasimkhan30
changed the title
PR Changes
|
Codecov Report❌ Patch coverage is ❌ Your patch status has failed because the patch coverage (61.49%) is below the target coverage (70.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #21467 +/- ##
==========================================
+ Coverage 74.06% 74.21% +0.14%
==========================================
Files 339 340 +1
Lines 102844 102895 +51
Branches 6029 6059 +30
==========================================
+ Hits 76173 76364 +191
+ Misses 26671 26531 -140
🚀 New features to boost your workflow:
|
…n configuration changes and add unit tests for the new behavior
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 22 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
aasimkhan30
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 23 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+4955
to
+4957
| <trans-unit id="++CODE++2ada9c77ff43c3a64eb670413987d527359f366fe2ddc314dfdfd6102df8ba07"> | ||
| <source xml:lang="en">Results Settings</source> | ||
| </trans-unit> |
There was a problem hiding this comment.
New trans-units were added/removed in the source vscode-mssql.xlf, but the corresponding localized XLIFF files (e.g. localization/xliff/vscode-mssql.de.xlf, etc.) still contain removed entries (like “Average: {0}”) and don’t include the new ones (like “Results Settings”). Please run yarn localization and commit the updated localization/xliff/*.xlf outputs so the repo’s localization files stay in sync.
Suggested change
| <trans-unit id="++CODE++2ada9c77ff43c3a64eb670413987d527359f366fe2ddc314dfdfd6102df8ba07"> | |
| <source xml:lang="en">Results Settings</source> | |
| </trans-unit> |
added 2 commits
March 10, 2026 15:44
…ummary # Conflicts: # extensions/mssql/src/controllers/mainController.ts # extensions/mssql/src/webviews/pages/QueryResult/queryResultSettingsControl.tsx # extensions/mssql/src/webviews/pages/QueryResult/queryResultSummaryFooter.tsx # localization/xliff/vscode-mssql.xlf
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 20 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
extensions/mssql/src/views/statusView.ts:37
FileStatusBarno longer definesrowCount/executionTime, but there are still references tobar.rowCountandbar.executionTimelater in this file (e.g., in the ownership-change handler). That will be a TypeScript compile error; remove/update those remainingshowStatusBarItemcalls (or reintroduce the status bar items if still needed).
public statusQuery: vscode.StatusBarItem;
// Item for language service status
public statusLanguageService: vscode.StatusBarItem;
// Item for SQLCMD Mode
public sqlCmdMode: vscode.StatusBarItem;
// Timer used for displaying a progress indicator on queries
public progressTimerId: NodeJS.Timeout;
public currentLanguageServiceStatus: string;
public queryTimer: NodeJS.Timeout;
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 21 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
extensions/mssql/src/webviews/pages/QueryResult/queryResultPane.tsx:207
- The keyboard shortcut handler effect captures
keyBindingsandisExecutionPlanbut they are missing from the dependency array. If keybindings are refreshed (or execution plan availability changes), the handler can use stale values and shortcuts may stop working as expected. Update the dependency list to include all referenced reactive values (and remove unrelated ones).
useEffect(() => {
const handler = (event: KeyboardEvent) => {
let handled = false;
if (
eventMatchesShortcut(
event,
keyBindings[WebviewAction.QueryResultSwitchToResultsTab]?.keyCombination,
)
) {
if (Object.keys(resultSetSummaries ?? {}).length > 0) {
context.setResultTab(qr.QueryResultPaneTabs.Results);
handled = true;
}
} else if (
eventMatchesShortcut(
event,
keyBindings[WebviewAction.QueryResultSwitchToMessagesTab]?.keyCombination,
)
) {
context.setResultTab(qr.QueryResultPaneTabs.Messages);
handled = true;
} else if (
eventMatchesShortcut(
event,
keyBindings[WebviewAction.QueryResultSwitchToQueryPlanTab]?.keyCombination,
)
) {
if (isExecutionPlan) {
context.setResultTab(qr.QueryResultPaneTabs.ExecutionPlan);
handled = true;
}
}
if (handled) {
event.preventDefault();
event.stopPropagation();
}
};
document.addEventListener("keydown", handler, true);
return () => {
document.removeEventListener("keydown", handler, true);
};
}, [tabStates?.resultPaneTab, getGridCount, context, resultSetSummaries]);
Comment on lines
799
to
+803
| // Handle cancellation from the progress dialog (user clicked cancel) | ||
| token?.onCancellationRequested(async () => { | ||
| await this._client.sendNotification(CancelCopy2Notification.type); | ||
| vscode.window.showInformationMessage("Copying results cancelled"); | ||
| resolve(); | ||
| resolve(false); |
There was a problem hiding this comment.
This user-facing message is not localized (hard-coded English). Please move it into the appropriate localization constants and use the localized string here.
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.
Description
Before:
Before: (with cell selection)
Now: (With cell selection)
Code Changes Checklist
npm run test)Reviewers: Please read our reviewer guidelines