history: ship review changes

Author: shakyShane

special-pages/pages/history/app/global/Providers/HistoryServiceProvider.js

@@ -13,7 +13,7 @@ import { generateHeights } from '../../utils.js';
  * | {kind: 'delete-domain'; domain: string }
  * | {kind: 'delete-entries-by-index'; value: number[] }
  * | {kind: 'open-url'; url: string, target: 'new-tab' | 'new-window' | 'same-tab' }
- * | {kind: 'show-entries-menu'; ids: string[]; indexes: number[] }
+ * | {kind: 'show-entries-menu'; indexes: number[] }
  * | {kind: 'request-more'; end: number }
  * } Action
  */
@@ -126,7 +126,7 @@ export function HistoryServiceProvider({ service, children, initial }) {
             }
             case 'show-entries-menu': {
                 service
-                    .entriesMenu(action.ids, action.indexes)
+                    .entriesMenu(action.indexes)
                     // eslint-disable-next-line promise/prefer-await-to-then
                     .then((resp) => {
                         if (resp.kind === 'domain-search') {

special-pages/pages/history/app/global/hooks/useButtonClickHandler.js

@@ -1,6 +1,7 @@
 import { useEffect } from 'preact/hooks';
 import { BTN_ACTION_ENTRIES_MENU, KNOWN_ACTIONS } from '../../constants.js';
 import { useHistoryServiceDispatch } from '../Providers/HistoryServiceProvider.js';
+import { useSelected } from '../Providers/SelectionProvider.js';
 
 /**
  * This function registers button click handlers that communicate with the history service.
@@ -11,6 +12,7 @@ import { useHistoryServiceDispatch } from '../Providers/HistoryServiceProvider.j
  */
 export function useButtonClickHandler() {
     const historyServiceDispatch = useHistoryServiceDispatch();
+    const selected = useSelected();
     useEffect(() => {
         function clickHandler(/** @type {MouseEvent} */ event) {
             if (!(event.target instanceof Element)) return;
@@ -30,11 +32,19 @@ export function useButtonClickHandler() {
 
             switch (action) {
                 case BTN_ACTION_ENTRIES_MENU: {
-                    historyServiceDispatch({
-                        kind: 'show-entries-menu',
-                        ids: [btn.value],
-                        indexes: [Number(btn.dataset.index)],
-                    });
+                    const index = parseInt(btn.dataset.index ?? '-1', 10);
+                    const withinSelection = selected.value.has(index);
+                    if (withinSelection) {
+                        historyServiceDispatch({
+                            kind: 'show-entries-menu',
+                            indexes: [...selected.value],
+                        });
+                    } else {
+                        historyServiceDispatch({
+                            kind: 'show-entries-menu',
+                            indexes: [Number(btn.dataset.index)],
+                        });
+                    }
                     return;
                 }
                 default:

special-pages/pages/history/app/global/hooks/useContextMenuForEntries.js

@@ -1,6 +1,6 @@
 import { useEffect } from 'preact/hooks';
 import { useSelected } from '../Providers/SelectionProvider.js';
-import { useHistoryServiceDispatch, useResultsData } from '../Providers/HistoryServiceProvider.js';
+import { useHistoryServiceDispatch } from '../Providers/HistoryServiceProvider.js';
 
 /**
  * Support for context menu on entries. This needs to be aware of
@@ -9,7 +9,6 @@ import { useHistoryServiceDispatch, useResultsData } from '../Providers/HistoryS
  */
 export function useContextMenuForEntries() {
     const selected = useSelected();
-    const results = useResultsData();
     const dispatch = useHistoryServiceDispatch();
 
     useEffect(() => {
@@ -26,18 +25,9 @@ export function useContextMenuForEntries() {
 
             const isSelected = elem.getAttribute('aria-selected') === 'true';
             if (isSelected) {
-                const indexes = [...selected.value];
-                const ids = [];
-                for (let i = 0; i < indexes.length; i++) {
-                    const current = results.value.items[indexes[i]];
-                    if (!current) throw new Error('unreachable');
-                    ids.push(current.id);
-                }
-                dispatch({ kind: 'show-entries-menu', ids, indexes });
+                dispatch({ kind: 'show-entries-menu', indexes: [...selected.value] });
             } else {
-                const button = /** @type {HTMLButtonElement|null} */ (elem.querySelector('button[value]'));
-                const id = button?.value || '';
-                dispatch({ kind: 'show-entries-menu', ids: [id], indexes: [Number(elem.dataset.index)] });
+                dispatch({ kind: 'show-entries-menu', indexes: [Number(elem.dataset.index)] });
             }
         }
 

special-pages/pages/history/app/history.service.js

@@ -216,12 +216,12 @@ export class HistoryService {
     }
 
     /**
-     * @param {string[]} ids
      * @param {number[]} indexes
      * @return {Promise<{kind: 'none'} | { kind: 'domain-search'; value: string }>}
      */
-    async entriesMenu(ids, indexes) {
-        console.log('📤 [entries_menu]: ', JSON.stringify({ ids }));
+    async entriesMenu(indexes) {
+        const ids = this._collectIds(indexes);
+        console.trace('📤 [entries_menu]: ', JSON.stringify({ ids }));
         const response = await this.history.messaging.request('entries_menu', { ids });
         if (response.action === 'none') {
             return { kind: 'none' };

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

@@ -187,4 +187,11 @@ test.describe('history selections', () => {
         await hp.selectsRowIndexWithShift(2);
         await hp.deletesWithKeyboard(hp.ids(3), { action: 'delete' });
     });
+    test('3 dots menu on multiple history entries', async ({ page }, workerInfo) => {
+        const hp = HistoryTestPage.create(page, workerInfo).withEntries(2000);
+        await hp.openPage({});
+        await hp.selectsRowIndex(0);
+        await hp.selectsRowIndexWithShift(2);
+        await hp.menuForMultipleHistoryEntries(0, hp.ids(3), { action: 'delete' });
+    });
 });

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

@@ -317,6 +317,27 @@ export class HistoryTestPage {
         cleanup();
     }
 
+    /**
+     * @param {number} nth - row index to click the 3 dots on
+     * @param {string[]} ids - expected ids
+     * @param {import('../types/history.ts').DeleteRangeResponse} resp
+     */
+    async menuForMultipleHistoryEntries(nth, ids, resp) {
+        const { page } = this;
+
+        const cleanup = this._withDialogHandling(resp);
+        // console.log(data[0].title);
+        const data = generateSampleData({ count: this.entries, offset: 0 });
+        const nthItem = data[nth];
+        const row = page.getByText(nthItem.title);
+        await row.hover();
+        await page.locator(`[data-action="entries_menu"][value=${nthItem.id}]`).click();
+
+        const calls = await this.mocks.waitForCallCount({ method: 'entries_menu', count: 1 });
+        expect(calls[0].payload.params).toStrictEqual({ ids: ids });
+        cleanup();
+    }
+
     /**
      * @param {number} nth
      */