diff --git a/ui/src/app/app.html b/ui/src/app/app.html index c6e436c..5707495 100644 --- a/ui/src/app/app.html +++ b/ui/src/app/app.html @@ -757,7 +757,7 @@ - + Video Type diff --git a/ui/src/app/app.ts b/ui/src/app/app.ts index d88cc9d..717daee 100644 --- a/ui/src/app/app.ts +++ b/ui/src/app/app.ts @@ -137,6 +137,9 @@ export class App implements AfterViewInit, OnInit, OnDestroy { sortAscending = false; expandedErrors: Set = new Set(); cachedSortedDone: [string, Download][] = []; + // The done ids in rendered order, so a shift-click range follows the sort + // the user is looking at rather than the map's insertion order. + cachedSortedDoneIds: string[] = []; lastCopiedErrorId: string | null = null; private previousDownloadType = 'video'; private addRequestSub?: Subscription; @@ -1532,6 +1535,7 @@ export class App implements AfterViewInit, OnInit, OnDestroy { result.reverse(); } this.cachedSortedDone = result; + this.cachedSortedDoneIds = result.map(([key]) => key); } toggleErrorDetail(id: string) { diff --git a/ui/src/app/components/master-checkbox.component.spec.ts b/ui/src/app/components/master-checkbox.component.spec.ts index 5c5202c..b302fba 100644 --- a/ui/src/app/components/master-checkbox.component.spec.ts +++ b/ui/src/app/components/master-checkbox.component.spec.ts @@ -2,6 +2,38 @@ import { TestBed } from '@angular/core/testing'; import { SelectAllCheckboxComponent } from './master-checkbox.component'; import { Checkable } from '../interfaces'; +function makeList(ids: string[]): Map { + const list = new Map(); + for (const id of ids) { + list.set(id, { checked: false }); + } + return list; +} + +function makeMaster(list: Map, orderedIds: string[] | null = null) { + const fixture = TestBed.createComponent(SelectAllCheckboxComponent); + fixture.componentRef.setInput('id', 'queue'); + fixture.componentRef.setInput('list', list); + if (orderedIds) { + fixture.componentRef.setInput('orderedIds', orderedIds); + } + fixture.detectChanges(); + return fixture; +} + +// Simulates what the item checkbox does: ngModel writes the new state, then +// the change handler reports the click to the master. +function clickItem( + master: SelectAllCheckboxComponent, + list: Map, + id: string, + shift = false, +) { + const item = list.get(id)!; + item.checked = !item.checked; + master.selectionChanged(id, shift); +} + describe('SelectAllCheckboxComponent', () => { beforeEach(async () => { await TestBed.configureTestingModule({ @@ -20,4 +52,87 @@ describe('SelectAllCheckboxComponent', () => { fixture.componentInstance.clicked(); expect(list.get('u1')?.checked).toBe(true); }); + + it('shift-click checks every item between the two clicks', () => { + const list = makeList(['u1', 'u2', 'u3', 'u4', 'u5']); + const master = makeMaster(list).componentInstance; + + clickItem(master, list, 'u2'); + clickItem(master, list, 'u4', true); + + expect([...list.values()].map((i) => i.checked)).toEqual([false, true, true, true, false]); + }); + + it('extends upwards as well as downwards', () => { + const list = makeList(['u1', 'u2', 'u3', 'u4']); + const master = makeMaster(list).componentInstance; + + clickItem(master, list, 'u4'); + clickItem(master, list, 'u2', true); + + expect([...list.values()].map((i) => i.checked)).toEqual([false, true, true, true]); + }); + + it('shift-clicking a checked box clears the range', () => { + const list = makeList(['u1', 'u2', 'u3']); + list.forEach((item) => (item.checked = true)); + const master = makeMaster(list).componentInstance; + + clickItem(master, list, 'u1'); + clickItem(master, list, 'u3', true); + + expect([...list.values()].map((i) => i.checked)).toEqual([false, false, false]); + }); + + it('follows the rendered order, not the map order', () => { + // The done list renders newest-first, so its rendered order is not the + // order the entries sit in the map. u2 lies inside the range on screen + // and outside it in the map, which is what separates the two. + const list = makeList(['u1', 'u2', 'u3', 'u4']); + const master = makeMaster(list, ['u4', 'u2', 'u3', 'u1']).componentInstance; + + clickItem(master, list, 'u4'); + clickItem(master, list, 'u3', true); + + // u1 (rendered last) stays clear; u2 is swept up with the range. + expect([...list.values()].map((i) => i.checked)).toEqual([false, true, true, true]); + }); + + it('a plain click after a range starts a new anchor', () => { + const list = makeList(['u1', 'u2', 'u3', 'u4']); + const master = makeMaster(list).componentInstance; + + clickItem(master, list, 'u1'); + clickItem(master, list, 'u2', true); + clickItem(master, list, 'u4'); + + expect([...list.values()].map((i) => i.checked)).toEqual([true, true, false, true]); + }); + + it('select-all clears the anchor so the next shift-click is a plain toggle', () => { + const list = makeList(['u1', 'u2', 'u3']); + const fixture = makeMaster(list); + const master = fixture.componentInstance; + + clickItem(master, list, 'u1'); + master.selected = true; + master.clicked(); + master.selected = false; + master.clicked(); + clickItem(master, list, 'u3', true); + + expect([...list.values()].map((i) => i.checked)).toEqual([false, false, true]); + }); + + it('ignores a range whose anchor row is gone', () => { + const list = makeList(['u1', 'u2', 'u3']); + const master = makeMaster(list).componentInstance; + + clickItem(master, list, 'u1'); + // The anchor finishes downloading and leaves the queue. + list.delete('u1'); + clickItem(master, list, 'u3', true); + + expect([...list.values()].map((i) => i.checked)).toEqual([false, true]); + }); }); diff --git a/ui/src/app/components/master-checkbox.component.ts b/ui/src/app/components/master-checkbox.component.ts index 6b4c147..985c13a 100644 --- a/ui/src/app/components/master-checkbox.component.ts +++ b/ui/src/app/components/master-checkbox.component.ts @@ -20,17 +20,33 @@ import { FormsModule } from "@angular/forms"; export class SelectAllCheckboxComponent { readonly id = input.required(); readonly list = input.required>(); + // The ids in the order the rows are rendered. The done list is sorted for + // display, so its order is not the map's insertion order, and a range + // selection has to follow what the user sees. Left unset, the map order is + // the rendered order. + readonly orderedIds = input(null); readonly changed = output(); readonly masterCheckbox = viewChild.required('masterCheckbox'); selected!: boolean; + // The item a range extends from: the last one toggled on its own. + private anchorId: string | null = null; + clicked() { this.list().forEach(item => item.checked = this.selected); + // Select-all is not a position, so there is nothing to extend from next. + this.anchorId = null; this.selectionChanged(); } - selectionChanged() { + selectionChanged(id?: string, extend = false) { + if (id !== undefined) { + if (extend && this.anchorId !== null && this.anchorId !== id) { + this.applyRange(this.anchorId, id); + } + this.anchorId = id; + } const masterCheckbox = this.masterCheckbox(); if (!masterCheckbox) return; @@ -40,4 +56,27 @@ export class SelectAllCheckboxComponent { masterCheckbox.nativeElement.indeterminate = checked > 0 && checked < this.list().size; this.changed.emit(checked); } + + // Everything between the anchor and the just-clicked row takes the state the + // click produced, so shift-clicking a checked box clears the range and + // shift-clicking an unchecked one fills it. + private applyRange(fromId: string, toId: string) { + const ids = this.orderedIds() ?? Array.from(this.list().keys()); + const from = ids.indexOf(fromId); + const to = ids.indexOf(toId); + // A row can disappear between two clicks (a download finishing moves it + // from the queue to the done list); without both ends there is no range. + if (from < 0 || to < 0) { + return; + } + const target = this.list().get(toId)?.checked ?? false; + const start = Math.min(from, to); + const end = Math.max(from, to); + for (let i = start; i <= end; i++) { + const item = this.list().get(ids[i]); + if (item) { + item.checked = target; + } + } + } } diff --git a/ui/src/app/components/slave-checkbox.component.spec.ts b/ui/src/app/components/slave-checkbox.component.spec.ts index 460b831..1866ee5 100644 --- a/ui/src/app/components/slave-checkbox.component.spec.ts +++ b/ui/src/app/components/slave-checkbox.component.spec.ts @@ -22,4 +22,33 @@ describe('ItemCheckboxComponent', () => { itemFixture.detectChanges(); expect(itemFixture.componentInstance).toBeTruthy(); }); + + it('reports the shift modifier from the click to the master', () => { + const masterFixture = TestBed.createComponent(SelectAllCheckboxComponent); + masterFixture.componentRef.setInput('id', 'q'); + masterFixture.componentRef.setInput('list', new Map()); + masterFixture.detectChanges(); + const master = masterFixture.componentInstance; + const reported: [string | undefined, boolean | undefined][] = []; + master.selectionChanged = (id?: string, extend?: boolean) => { + reported.push([id, extend]); + }; + + const itemFixture = TestBed.createComponent(ItemCheckboxComponent); + itemFixture.componentRef.setInput('id', 'row1'); + itemFixture.componentRef.setInput('master', master); + itemFixture.componentRef.setInput('checkable', { checked: false }); + itemFixture.detectChanges(); + const item = itemFixture.componentInstance; + + item.clicked(new MouseEvent('click', { shiftKey: true })); + item.changed(); + // The modifier must not stick to the next toggle. + item.changed(); + + expect(reported).toEqual([ + ['row1', true], + ['row1', false], + ]); + }); }); diff --git a/ui/src/app/components/slave-checkbox.component.ts b/ui/src/app/components/slave-checkbox.component.ts index 4100a16..4371cb3 100644 --- a/ui/src/app/components/slave-checkbox.component.ts +++ b/ui/src/app/components/slave-checkbox.component.ts @@ -7,14 +7,14 @@ import { FormsModule } from '@angular/forms'; selector: 'app-item-checkbox', template: `
- +
`, // Shared Checkable objects are mutated in place; Eager preserves pre-v22 behavior. // eslint-disable-next-line @angular-eslint/prefer-on-push-component-change-detection changeDetection: ChangeDetectionStrategy.Eager, - imports: [ + imports: [ FormsModule ] }) @@ -22,4 +22,19 @@ export class ItemCheckboxComponent { readonly id = input.required(); readonly master = input.required(); readonly checkable = input.required(); + + // click fires before change, so the modifier is recorded here and read once + // ngModel has written the new state into the checkable. Keyboard activation + // fires change without a click, which is a plain toggle. + private extend = false; + + clicked(event: MouseEvent) { + this.extend = event.shiftKey; + } + + changed() { + const extend = this.extend; + this.extend = false; + this.master().selectionChanged(this.id(), extend); + } }