Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Select all projects iconbutton #574

Merged
merged 9 commits into from
Dec 20, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions media/settingsView.css
Original file line number Diff line number Diff line change
Expand Up @@ -80,3 +80,12 @@ body[data-vscode-theme-kind=vscode-dark] div.separator {
width: 4px;
height: 4px;
}

.section-toolbar > a {
border-radius: 5px;
font-size: 16px;
}

.section-toolbar > a:hover {
background-color: var(--vscode-toolbar-hoverBackground);
}
6 changes: 5 additions & 1 deletion media/theme.css
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,11 @@ textarea {

.section-header {
font-size: 11px;
margin: 10px 0 3px 8px;
margin: 10px 8px 3px 8px;
font-weight: 700;
color: var(--vscode-editor-inlineValuesForeground);

display: flex;
justify-content: space-between;
align-items: baseline;
}
31 changes: 29 additions & 2 deletions src/settingsView.ts
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,17 @@ function htmlForWebview(vscode: vscodeTypes.VSCode, extensionUri: vscodeTypes.Ur
<span id="configToolbar"></span>
</div>
</div>
<div class="section-header">${vscode.l10n.t('PROJECTS')}</div>
<div class="section-header">
${vscode.l10n.t('PROJECTS')}
<div class="section-toolbar">
<a id="selectAll" role="button" title="Select All">
<svg width="48" height="48" viewBox="0 0 16 16" xmlns="http://www.w3.org/2000/svg" fill="currentColor"><path d="M9 9H4v1h5V9z"/><path d="M7 12V7H6v5h1z"/><path fill-rule="evenodd" clip-rule="evenodd" d="M5 3l1-1h7l1 1v7l-1 1h-2v2l-1 1H3l-1-1V6l1-1h2V3zm1 2h4l1 1v4h2V3H6v2zm4 1H3v7h7V6z"/></svg>
</a>
<a id="unselectAll" role="button" title="Unselect All" hidden>
<svg width="48" height="48" viewBox="0 0 16 16" xmlns="http://www.w3.org/2000/svg" fill="currentColor"><path d="M9 9H4v1h5V9z"/><path fill-rule="evenodd" clip-rule="evenodd" d="M5 3l1-1h7l1 1v7l-1 1h-2v2l-1 1H3l-1-1V6l1-1h2V3zm1 2h4l1 1v4h2V3H6v2zm4 1H3v7h7V6z"/></svg>
</a>
</div>
</div>
<div data-testid="projects" id="projects" class="list"></div>
<div class="section-header">${vscode.l10n.t('SETTINGS')}</div>
<div class="list">
Expand Down Expand Up @@ -299,9 +309,12 @@ function htmlForWebview(vscode: vscodeTypes.VSCode, extensionUri: vscodeTypes.Ur
<div id="rareActions" class="list"></div>
</body>
<script nonce="${nonce}">
const projectsElement = document.getElementById('projects');
const selectAllButton = document.getElementById('selectAll');
const unselectAllButton = document.getElementById('unselectAll');

let selectConfig;
function updateProjects(projects) {
const projectsElement = document.getElementById('projects');
projectsElement.textContent = '';
for (const project of projects) {
const { name, enabled } = project;
Expand All @@ -318,7 +331,21 @@ function htmlForWebview(vscode: vscodeTypes.VSCode, extensionUri: vscodeTypes.Ur
div.appendChild(label);
projectsElement.appendChild(div);
}

const allEnabled = projects.every(p => p.enabled);
selectAllButton.hidden = allEnabled;
unselectAllButton.hidden = !allEnabled;
}

function setAllProjects(checked) {
for (const input of projectsElement.querySelectorAll('input[type=checkbox]')) {
input.checked = checked;
input.dispatchEvent(new Event('change'));
Copy link
Member

Choose a reason for hiding this comment

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

This would do a lot of things many times if you have 20 projects. I'd recommend making it a bit more MVC-y:

Option 1:
You currently dispatch an event in line 343 that you listen to in line 327. You can consider short-cutting it and making setProjectEnabled receive array of values for a batch update. That way instead of 20 whole-UI updates we get 1.

Option 2:
Going even further MVC where sidebar is a View, introduce an event "setAllProjectEnabled(boolean)" that would site next to setProjectEnabled and would do the right thing, also once.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good. I implemented Option 2 in 8f2b484 - Option 1 would've required keeping a list of project names within the view, which felt weird.

}
}

selectAllButton.addEventListener('click', () => setAllProjects(true));
unselectAllButton.addEventListener('click', () => setAllProjects(false));

const vscode = acquireVsCodeApi();
for (const input of document.querySelectorAll('input[type=checkbox]')) {
Expand Down
46 changes: 46 additions & 0 deletions tests/settings.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,52 @@ test('should toggle setting from webview', async ({ activate }) => {
expect(configuration.get('reuseBrowser')).toBe(true);
});

test('should select-all/unselect-all', async ({ activate }) => {
const { vscode } = await activate({
'playwright.config.ts': `
import { defineConfig } from '@playwright/test';
export default defineConfig({
projects: [
{ name: 'foo', testMatch: 'foo.ts' },
{ name: 'bar', testMatch: 'bar.ts' },
]
});
`,
});

const webView = vscode.webViews.get('pw.extension.settingsView')!;

await expect(webView.locator('body')).toMatchAriaSnapshot(`
- text: PROJECTS
- button "Select All"
- checkbox "foo" [checked]
- checkbox "bar" [checked=false]
`);

await webView.getByRole('checkbox', { name: 'bar' }).check();

await expect(webView.locator('body')).toMatchAriaSnapshot(`
- button "Unselect All"
- checkbox "foo" [checked]
- checkbox "bar" [checked]
`);

await webView.getByRole('button', { name: 'Unselect All' }).click();

await expect(webView.locator('body')).toMatchAriaSnapshot(`
- button "Select All"
- checkbox "foo" [checked=false]
- checkbox "bar" [checked=false]
`);

await webView.getByRole('button', { name: 'Select All' }).click();
await expect(webView.locator('body')).toMatchAriaSnapshot(`
- button "Unselect All"
- checkbox "foo" [checked]
- checkbox "bar" [checked]
`);
});

test('should reflect changes to setting', async ({ activate }) => {
const { vscode } = await activate({
'playwright.config.js': `module.exports = {}`,
Expand Down
Loading