smarter delete-all button in header

Author: shakyShane

special-pages/pages/history/app/components/Header.js

@@ -14,10 +14,11 @@ import { useHistoryServiceDispatch } from '../global-state/HistoryServiceProvide
 export function Header() {
     const search = useQueryContext();
     const term = useComputed(() => search.value.term);
+    const range = useComputed(() => search.value.range);
     const domain = useComputed(() => search.value.domain);
     return (
         <div class={styles.root}>
-            <Controls term={term} />
+            <Controls term={term} range={range} />
             <div class={styles.search}>
                 <SearchForm term={term} domain={domain} />
             </div>
@@ -30,22 +31,52 @@ export function Header() {
  *
  * @param {Object} props - Properties passed to the component.
  * @param {import("@preact/signals").Signal<string|null>} props.term
+ * @param {import("@preact/signals").Signal<string|null>} props.range
  */
-function Controls({ term }) {
+function Controls({ term, range }) {
     const { t } = useTypedTranslation();
+    const { results } = useData();
     const selected = useSelected();
     const dispatch = useHistoryServiceDispatch();
+
+    /**
+     * Aria labels + title text is derived from the current result set.
+     */
+    const ariaDisabled = useComputed(() => (results.value.items.length === 0 ? 'true' : 'false'));
+    const title = useComputed(() => (results.value.items.length === 0 ? t('delete_none') : ''));
+
+    /**
+     * The button text should alternate between 'delete' and 'delete all' depending on the
+     * state of the current query. It should only read 'delete all' when the query is the default one
+     * and there are no selections
+     */
     const buttonTxt = useComputed(() => {
         const hasTerm = term.value !== null && term.value.trim() !== '';
         const hasSelections = selected.value.size > 0;
-        return hasTerm || hasSelections ? t('delete_some') : t('delete_all');
+        const hasRange = range.value !== null;
+        return hasTerm || hasSelections || hasRange ? t('delete_some') : t('delete_all');
     });
-    const { results } = useData();
-    const ariaDisabled = useComputed(() => (results.value.items.length === 0 ? 'true' : 'false'));
-    const title = useComputed(() => (results.value.items.length === 0 ? t('delete_none') : ''));
+
+    /**
+     * Which action should the delete button take?
+     *
+     * - if there are selections, they should be deleted
+     * - if there's a range selected, that should be deleted
+     * - or fallback to deleting all
+     */
+    function onClick() {
+        if (selected.value.size > 0) {
+            return dispatch({ kind: 'delete-entries-by-index', value: [...selected.value] });
+        }
+        if (range.value !== null) {
+            return dispatch({ kind: 'delete-range', value: range.value });
+        }
+        dispatch({ kind: 'delete-all' });
+    }
+
     return (
         <div class={styles.controls}>
-            <button class={styles.largeButton} onClick={() => dispatch({ kind: 'delete-all' })} aria-disabled={ariaDisabled} title={title}>
+            <button class={styles.largeButton} onClick={onClick} aria-disabled={ariaDisabled} title={title}>
                 <Trash />
                 <span>{buttonTxt}</span>
             </button>

special-pages/pages/history/integration-tests/history-selections.spec.js

@@ -135,6 +135,13 @@ test.describe('history selections', () => {
         await hp.selectsRowIndex(0);
         await hp.deleteAllButtonReflectsSelection();
     });
+    test('`delete` in header respects selection', async ({ page }, workerInfo) => {
+        const hp = HistoryTestPage.create(page, workerInfo).withEntries(2000);
+        await hp.openPage({});
+        await hp.selectsRowIndex(0);
+        await hp.clicksDeleteInHeader({ action: 'delete' });
+        await hp.didDeleteSelection([0]);
+    });
     test('`deleteAll` button text changes when a query is present', async ({ page }, workerInfo) => {
         const hp = HistoryTestPage.create(page, workerInfo).withEntries(2000);
         await hp.openPage({});

special-pages/pages/history/integration-tests/history.page.js

@@ -227,6 +227,14 @@ export class HistoryTestPage {
         expect(calls[0].payload.params).toStrictEqual({ range: 'today' });
     }
 
+    /**
+     * @param {import("../types/history.js").Range} range
+     */
+    async didDeleteRange(range) {
+        const calls = await this.mocks.waitForCallCount({ method: 'deleteRange', count: 1 });
+        expect(calls[0].payload.params).toStrictEqual({ range });
+    }
+
     /**
      * @param {import('../types/history.ts').DeleteRangeResponse} resp
      */
@@ -259,6 +267,15 @@ export class HistoryTestPage {
         await expect(page.getByRole('heading', { level: 2, name: 'Nothing to see here!' })).toBeVisible();
     }
 
+    /**
+     * @param {import('../types/history.ts').DeleteRangeResponse} resp
+     */
+    async clicksDeleteInHeader(resp) {
+        const { page } = this;
+        this._withDialogHandling(resp);
+        await page.locator('header').getByRole('button', { name: 'Delete', exact: true }).click();
+    }
+
     /**
      * @param {import('../types/history.ts').DeleteRangeResponse} resp
      */
@@ -502,4 +519,14 @@ export class HistoryTestPage {
         // await this.page.pause();
         await this.page.getByRole('button', { name: `Delete history for ${range}` }).hover();
     }
+
+    /**
+     * @param {number[]} indexes
+     */
+    async didDeleteSelection(indexes) {
+        const items = generateSampleData({ count: this.entries, offset: 0 });
+        const ids = indexes.map((index) => items[index].id);
+        const calls = await this.mocks.waitForCallCount({ method: 'entries_delete', count: 1 });
+        expect(calls[0].payload.params).toStrictEqual({ ids });
+    }
 }

special-pages/pages/history/integration-tests/history.spec.js

@@ -116,6 +116,13 @@ test.describe('history', () => {
         await hp.openPage({});
         await hp.deletesAllHistoryFromHeader({ action: 'delete' });
     });
+    test('deleting range from the header', async ({ page }, workerInfo) => {
+        const hp = HistoryTestPage.create(page, workerInfo).withEntries(2000);
+        await hp.openPage({});
+        await hp.selectsToday();
+        await hp.clicksDeleteInHeader({ action: 'delete' });
+        await hp.didDeleteRange('today');
+    });
     test('3 dots menu on Section title', async ({ page }, workerInfo) => {
         const hp = HistoryTestPage.create(page, workerInfo).withEntries(2000);
         await hp.openPage({});