Skip to content

Conversation

@ghiscoding
Copy link
Contributor

@ghiscoding ghiscoding commented Nov 27, 2025

Pull Request Template – vscode-mssql

Description

a simple follow-up PR to previous PR #20694, most specifically a follow-up to this comment

it merges 3 DOM elements into a single one since they all follow roughly the same logic.

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

Future Code Review Questions

@lewis-sanchez I see that you added the editCommandHandler which is typically used for Undo Edit, but in your case it looks like you provided your own way of dealing with Undo... so in that case the use of editCommandHandler is totally irrelevant because editCommand.execute(); is already the default code execution within SlickGrid when no edit handler is provided (see here), which mean that you could remove this handler without affecting the code (unless you really want to use its Undo feature later on?)

editCommandHandler: (_item, _column, editCommand) => {
editCommand.execute();
},

@codecov-commenter
Copy link

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.83%. Comparing base (55a828d) to head (585f49b).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main   #20702   +/-   ##
=======================================
  Coverage   64.83%   64.83%           
=======================================
  Files         210      210           
  Lines       19443    19443           
  Branches     2472     2472           
=======================================
  Hits        12606    12606           
  Misses       6752     6752           
  Partials       85       85           
🚀 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
Copy link
Contributor

Hi @ghiscoding, thank you for contributing this pull request.

You're right about editCommandHandler being unnecessary in tableDataGrid.tsx. I will clean that up in a follow up PR.

@lewis-sanchez lewis-sanchez merged commit 8b5157e into microsoft:main Dec 1, 2025
1 of 2 checks passed
@lewis-sanchez
Copy link
Contributor

There's a 403 error in this pull request too, but it's not related to any of your changes. I verified your changes and they look good. Thanks again!

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.

3 participants