ntp: send source index in move action (#1250)

* ntp: send source index in move action * cleanup interface * linting --------- Co-authored-by: Shane Osbourne <sosbourne@duckduckgo.com>
duckduckgo · Nov 21, 2024 · 3e17e6f · 3e17e6f
1 parent e891989
commit 3e17e6f
Show file tree

Hide file tree

Showing 8 changed files with 45 additions and 16 deletions.
diff --git a/special-pages/messages/new-tab/favorites_move.notify.json b/special-pages/messages/new-tab/favorites_move.notify.json
@@ -4,6 +4,7 @@
   "type": "object",
   "required": [
     "id",
+    "fromIndex",
     "targetIndex"
   ],
   "properties": {
@@ -14,6 +15,10 @@
     "targetIndex": {
       "description": "zero-indexed target",
       "type": "number"
+    },
+    "fromIndex": {
+      "description": "zero-indexed source",
+      "type": "number"
     }
   }
 }
diff --git a/special-pages/pages/new-tab/app/favorites/components/FavoritesProvider.js b/special-pages/pages/new-tab/app/favorites/components/FavoritesProvider.js
@@ -12,6 +12,12 @@ import { reducer, useConfigSubscription, useDataSubscription, useInitialDataAndC
  * @typedef {import('../../../../../types/new-tab.ts').FavoritesOpenAction['target']} OpenTarget
  * @typedef {import('../../service.hooks.js').State<FavoritesData, FavoritesConfig>} State
  * @typedef {import('../../service.hooks.js').Events<FavoritesData, FavoritesConfig>} Events
+ * @typedef {{id: string; url: string}} BaseFavoriteType
+ */
+
+/**
+ * @template {BaseFavoriteType} ItemType - allow any type that extends BaseFavoriteType
+ * @typedef {(params: { list: ItemType[], id: string, fromIndex: number, targetIndex: number }) => void} ReorderFn
  */
 
 /**
@@ -24,8 +30,8 @@ export const FavoritesContext = createContext({
     toggle: () => {
         throw new Error('must implement');
     },
-    /** @type {(list: Favorite[], id: string, targetIndex: number) => void} */
-    favoritesDidReOrder: (list, id, targetIndex) => {
+    /** @type {ReorderFn<Favorite>} */
+    favoritesDidReOrder: ({ list, id, fromIndex, targetIndex }) => {
         throw new Error('must implement');
     },
     /** @type {(id: string) => void} */
@@ -68,11 +74,11 @@ export function FavoritesProvider({ children }) {
     // subscribe to toggle + expose a fn for sync toggling
     const { toggle } = useConfigSubscription({ dispatch, service });
 
-    /** @type {(f: Favorite[], id: string, targetIndex: number) => void} */
+    /** @type {ReorderFn<Favorite>} */
     const favoritesDidReOrder = useCallback(
-        (favorites, id, targetIndex) => {
+        ({ list, id, fromIndex, targetIndex }) => {
             if (!service.current) return;
-            service.current.setFavoritesOrder({ favorites }, id, targetIndex);
+            service.current.setFavoritesOrder({ favorites: list }, id, fromIndex, targetIndex);
         },
         [service],
     );

diff --git a/special-pages/pages/new-tab/app/favorites/components/PragmaticDND.js b/special-pages/pages/new-tab/app/favorites/components/PragmaticDND.js
@@ -24,7 +24,7 @@ const InstanceIdContext = createContext(getInstanceId());
  * @param {object} props
  * @param {import("preact").ComponentChild} props.children
  * @param {T[]} props.items
- * @param {(list: T[], id: string, index: number) => void} props.itemsDidReOrder
+ * @param {import('./FavoritesProvider.js').ReorderFn<{id: string; url: string}>} props.itemsDidReOrder
  */
 export function PragmaticDND({ children, items, itemsDidReOrder }) {
     /**
@@ -43,7 +43,7 @@ export function PragmaticDND({ children, items, itemsDidReOrder }) {
 /**
  * @template {{id: string; url: string}} T
  * @param {T[]} favorites
- * @param {(items: T[], id: string, target: number) => void} itemsDidReOrder
+ * @param {import('./FavoritesProvider.js').ReorderFn<{id: string; url: string}>} itemsDidReOrder
  * @param {symbol} instanceId
  */
 function useGridState(favorites, itemsDidReOrder, instanceId) {
@@ -89,7 +89,12 @@ function useGridState(favorites, itemsDidReOrder, instanceId) {
                         axis: 'horizontal',
                     });
 
-                    itemsDidReOrder(favorites, id, targetIndex);
+                    itemsDidReOrder({
+                        list: favorites,
+                        id,
+                        fromIndex: favorites.length,
+                        targetIndex,
+                    });
                 },
             }),
             monitorForElements({
@@ -147,7 +152,12 @@ function useGridState(favorites, itemsDidReOrder, instanceId) {
 
                     flushSync(() => {
                         try {
-                            itemsDidReOrder(reorderedList, startId, targetIndex);
+                            itemsDidReOrder({
+                                list: reorderedList,
+                                id: startId,
+                                fromIndex: startIndex,
+                                targetIndex,
+                            });
                         } catch (e) {
                             console.error('did catch', e);
                         }

diff --git a/special-pages/pages/new-tab/app/favorites/favorites.service.js b/special-pages/pages/new-tab/app/favorites/favorites.service.js
@@ -83,9 +83,10 @@ export class FavoritesService {
      * @param {FavoritesData} data
      * @param {string} id - entity id to move
      * @param {number} targetIndex - target index
+     * @param {number} fromIndex - from index
      * @internal
      */
-    setFavoritesOrder(data, id, targetIndex) {
+    setFavoritesOrder(data, id, fromIndex, targetIndex) {
         // update in memory instantly - this will broadcast changes to all listeners
 
         this.dataService.update((_old) => {
@@ -96,6 +97,7 @@ export class FavoritesService {
         this.ntp.messaging.notify('favorites_move', {
             id,
             targetIndex,
+            fromIndex,
         });
     }
 

diff --git a/special-pages/pages/new-tab/app/favorites/integration-tests/favorites.page.js b/special-pages/pages/new-tab/app/favorites/integration-tests/favorites.page.js
@@ -157,13 +157,13 @@ export class FavoritesPage {
         return { id };
     }
 
-    async sent({ id, targetIndex }) {
+    async sent({ id, fromIndex, targetIndex }) {
         const calls = await this.ntp.mocks.waitForCallCount({ method: 'favorites_move', count: 1 });
         expect(calls[0].payload).toStrictEqual({
             context: 'specialPages',
             featureName: 'newTabPage',
             method: 'favorites_move',
-            params: { id, targetIndex },
+            params: { id, fromIndex, targetIndex },
         });
     }
 
@@ -259,6 +259,7 @@ export class FavoritesPage {
             params: {
                 id: '3',
                 targetIndex: index,
+                fromIndex: 15, // this is the length of the list, and it gets dropped at the end in the test.
             },
         });
     }

diff --git a/special-pages/pages/new-tab/app/favorites/integration-tests/favorites.spec.js b/special-pages/pages/new-tab/app/favorites/integration-tests/favorites.spec.js
@@ -78,7 +78,7 @@ test.describe('newtab favorites', () => {
         await ntp.reducedMotion();
         await ntp.openPage();
         const { id } = await favorites.drags({ index: 0, to: 2 });
-        await favorites.sent({ id, targetIndex: 2 });
+        await favorites.sent({ id, fromIndex: 0, targetIndex: 2 });
     });
     test('support drop on placeholders', async ({ page }, workerInfo) => {
         const ntp = NewtabPage.create(page, workerInfo);
@@ -93,7 +93,7 @@ test.describe('newtab favorites', () => {
         const PLACEHOLDER_INDEX = 4;
         const EXPECTED_TARGET_INDEX = 2;
         const { id } = await favorites.drags({ index: 0, to: PLACEHOLDER_INDEX });
-        await favorites.sent({ id, targetIndex: EXPECTED_TARGET_INDEX });
+        await favorites.sent({ id, fromIndex: 0, targetIndex: EXPECTED_TARGET_INDEX });
     });
     test('accepts external drag/drop', async ({ page }, workerInfo) => {
         const ntp = NewtabPage.create(page, workerInfo);

diff --git a/special-pages/pages/new-tab/app/favorites/mocks/MockFavoritesProvider.js b/special-pages/pages/new-tab/app/favorites/mocks/MockFavoritesProvider.js
@@ -45,8 +45,9 @@ export function MockFavoritesProvider({ data = favorites.many, config = DEFAULT_
         }
     }, [state.status, state.config?.expansion, isReducedMotion]);
 
-    const favoritesDidReOrder = useCallback((/** @type {Favorite[]} */ newList) => {
-        dispatch({ kind: 'data', data: { favorites: newList } });
+    /** @type {import('../components/FavoritesProvider.js').ReorderFn<Favorite>} */
+    const favoritesDidReOrder = useCallback(({ list }) => {
+        dispatch({ kind: 'data', data: { favorites: list } });
     }, []);
 
     const openContextMenu = (...args) => {

diff --git a/special-pages/types/new-tab.ts b/special-pages/types/new-tab.ts
@@ -120,6 +120,10 @@ export interface FavoritesMoveAction {
    * zero-indexed target
    */
   targetIndex: number;
+  /**
+   * zero-indexed source
+   */
+  fromIndex: number;
 }
 /**
  * Generated from @see "../messages/new-tab/favorites_open.notify.json"