feat: Select all projects checkbox #569
Conversation
There was a problem hiding this comment.
Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (3)
src/settingsView.ts:304
- The variable 'selectConfig' is declared but never used. It should be removed to clean up the code.
let selectConfig;
src/settingsView.ts:301
- The 'projectsElement' and 'selectAllCheckbox' should be defined before they are used in the 'updateProjects' function to avoid potential issues.
const projectsElement = document.getElementById('projects');
src/settingsView.ts:337
- The 'updateProjects' function should have test coverage to ensure the 'selectAllCheckbox' behavior is correctly implemented.
selectAllCheckbox.addEventListener('change', event => {
src/settingsView.ts
Outdated
| @@ -316,12 +319,34 @@ function htmlForWebview(vscode: vscodeTypes.VSCode, extensionUri: vscodeTypes.Ur | |||
| div.appendChild(label); | |||
| projectsElement.appendChild(div); | |||
| } | |||
|
|
|||
| const allEnabled = projects.every(p => p.enabled); | |||
There was a problem hiding this comment.
You set this to a variable, then never use it, and redo the same calculation in the next line.
src/settingsView.ts
Outdated
| const vscode = acquireVsCodeApi(); | ||
| for (const input of document.querySelectorAll('input[type=checkbox]')) { | ||
| input.addEventListener('change', event => { | ||
| vscode.postMessage({ method: 'toggle', params: { setting: event.target.getAttribute('setting') } }); | ||
| const setting = event.target.getAttribute('setting'); | ||
| if (setting) |
There was a problem hiding this comment.
What happens if there's no setting (I don't know this VSCode API)?
There was a problem hiding this comment.
It's standard DOM. We have some input elements in the HTML that have a setting attribute, e.g. this:
Before this change, there were no elements without a setting. But i've added a new input element that doesn't have it, so we need to check this here. Makes me think we shouldn't attach the event listener in the first place, letb me change that!
There was a problem hiding this comment.
adding it to the css selector is smoother I think: a1e1a82
Works towards microsoft/playwright#33865. Once we have a "unselect all" button, we can automatically select all projects by default.
Screen.Recording.2024-12-13.at.15.30.12.mov