fix(insights): user-defined send/bindEvent overrides internal click (#…

…5527) * fix(insights): prevent duplicate events in sendEvent, we mark internal events (click and conversion), and prevent them being sent if there was a custom click right before. Note that this doesn't yet work with bindEvent, as it's higher in the bubbling tree * fix(internal): bail out of default click if element has a bindEvent parent * test * fix: only prevent next clicks after a regular click * bundlesize * test(infiniteHits): add sendEvent tests same idea as this test in hits-integration-test.ts * wip infinite hits * move bindEvent click listener people can't bind events higher anyway, so this is safe to do * refactor: move dedupe into middleware this allows the dedupe to work for both bindEvent and sendEvent in a single codebase. Tiny aside: this means that the "next internal click" is now global, meaning if you click on geo, then on hits, we'll see that as an internal click to prevent sending. This is an extreme edge case, as almost nowhere sends click events. * fix(events): move back inside connector this prevents a custom blocking an internal in a different widget * get rid of global cruft * refactor: use timers instead of order-based ignoring * test(common): insights clicks + view for hits + infinitehits * fix(react): send an internal event * PR feedback * internal
algolia · Apr 18, 2023 · bc69429 · bc69429
1 parent 45f9e01
commit bc69429
Show file tree

Hide file tree

Showing 23 changed files with 1,906 additions and 125 deletions.
diff --git a/bundlesize.config.json b/bundlesize.config.json
@@ -26,11 +26,11 @@
     },
     {
       "path": "packages/react-instantsearch-hooks/dist/umd/ReactInstantSearchHooks.min.js",
-      "maxSize": "43 kB"
+      "maxSize": "43.25 kB"
     },
     {
       "path": "packages/react-instantsearch-hooks-web/dist/umd/ReactInstantSearchHooksDOM.min.js",
-      "maxSize": "52.50 kB"
+      "maxSize": "52.75 kB"
     },
     {
       "path": "packages/react-instantsearch-dom/dist/umd/ReactInstantSearchDOM.min.js",
@@ -42,11 +42,11 @@
     },
     {
       "path": "packages/vue-instantsearch/vue2/umd/index.js",
-      "maxSize": "61.25 kB"
+      "maxSize": "61.50 kB"
     },
     {
       "path": "packages/vue-instantsearch/vue3/umd/index.js",
-      "maxSize": "61.25 kB"
+      "maxSize": "61.75 kB"
     },
     {
       "path": "packages/vue-instantsearch/vue2/cjs/index.js",

diff --git a/packages/instantsearch.js/src/__tests__/common.test.tsx b/packages/instantsearch.js/src/__tests__/common.test.tsx
@@ -8,6 +8,7 @@ import {
   createPaginationTests,
   createMenuTests,
   createInfiniteHitsTests,
+  createHitsTests,
 } from '@instantsearch/tests';
 
 import instantsearch from '../index.es';
@@ -19,6 +20,8 @@ import {
   pagination,
   infiniteHits,
   searchBox,
+  hits,
+  index,
 } from '../widgets';
 
 createHierarchicalMenuTests(({ instantSearchOptions, widgetParams }) => {
@@ -120,9 +123,117 @@ createInfiniteHitsTests(({ instantSearchOptions, widgetParams }) => {
         container: document.body.appendChild(document.createElement('div')),
       }),
       infiniteHits({
+        container: document.body.appendChild(
+          Object.assign(document.createElement('div'), {
+            id: 'main-hits',
+          })
+        ),
+        templates: {
+          item: (hit, { html, sendEvent }) =>
+            html`<div data-testid=${`main-hits-top-level-${hit.__position}`}>
+              ${hit.objectID}
+              <button
+                data-testid=${`main-hits-convert-${hit.__position}`}
+                onClick=${() => sendEvent('conversion', hit, 'Converted')}
+              >
+                convert
+              </button>
+              <button
+                data-testid=${`main-hits-click-${hit.__position}`}
+                onClick=${() => sendEvent('click', hit, 'Clicked')}
+              >
+                click
+              </button>
+            </div>`,
+        },
+        ...widgetParams,
+      }),
+      index({ indexName: 'nested' }).addWidgets([
+        infiniteHits({
+          container: document.body.appendChild(
+            Object.assign(document.createElement('div'), {
+              id: 'nested-hits',
+            })
+          ),
+          templates: {
+            item: (hit, { html, sendEvent }) =>
+              html`<div
+                data-testid=${`nested-hits-top-level-${hit.__position}`}
+              >
+                ${hit.objectID}
+                <button
+                  data-testid=${`nested-hits-click-${hit.__position}`}
+                  onClick=${() => sendEvent('click', hit, 'Clicked nested')}
+                >
+                  click
+                </button>
+              </div>`,
+          },
+        }),
+      ]),
+    ])
+    .on('error', () => {
+      /*
+       * prevent rethrowing InstantSearch errors, so tests can be asserted.
+       * IRL this isn't needed, as the error doesn't stop execution.
+       */
+    })
+    .start();
+});
+
+createHitsTests(({ instantSearchOptions, widgetParams }) => {
+  instantsearch(instantSearchOptions)
+    .addWidgets([
+      searchBox({
         container: document.body.appendChild(document.createElement('div')),
+      }),
+      hits({
+        container: document.body.appendChild(
+          Object.assign(document.createElement('div'), {
+            id: 'main-hits',
+          })
+        ),
+        templates: {
+          item: (hit, { html, sendEvent }) =>
+            html`<div data-testid=${`main-hits-top-level-${hit.__position}`}>
+              ${hit.objectID}
+              <button
+                data-testid=${`main-hits-convert-${hit.__position}`}
+                onClick=${() => sendEvent('conversion', hit, 'Converted')}
+              >
+                convert
+              </button>
+              <button
+                data-testid=${`main-hits-click-${hit.__position}`}
+                onClick=${() => sendEvent('click', hit, 'Clicked')}
+              >
+                click
+              </button>
+            </div>`,
+        },
         ...widgetParams,
       }),
+      index({ indexName: 'nested' }).addWidgets([
+        hits({
+          container: document.body.appendChild(
+            Object.assign(document.createElement('div'), {
+              id: 'nested-hits',
+            })
+          ),
+          templates: {
+            item: (hit, { html, sendEvent }) =>
+              html`<div data-testid=${`nested-hits-${hit.__position}`}>
+                ${hit.objectID}
+                <button
+                  data-testid=${`nested-hits-click-${hit.__position}`}
+                  onClick=${() => sendEvent('click', hit, 'Clicked nested')}
+                >
+                  click
+                </button>
+              </div>`,
+          },
+        }),
+      ]),
     ])
     .on('error', () => {
       /*

diff --git a/packages/instantsearch.js/src/components/Hits/Hits.tsx b/packages/instantsearch.js/src/components/Hits/Hits.tsx
@@ -55,7 +55,7 @@ export default function Hits({
   }
 
   return (
-    <div className={cssClasses.root} onClick={handleInsightsClick}>
+    <div className={cssClasses.root}>
       <ol className={cssClasses.list}>
         {hits.map((hit, index) => (
           <Template
@@ -64,8 +64,9 @@ export default function Hits({
             rootTagName="li"
             rootProps={{
               className: cssClasses.item,
-              onClick: () => {
-                sendEvent('click', hit, 'Hit Clicked');
+              onClick: (event: MouseEvent) => {
+                handleInsightsClick(event);
+                sendEvent('click:internal', hit, 'Hit Clicked');
               },
             }}
             key={hit.objectID}

diff --git a/packages/instantsearch.js/src/components/Hits/__tests__/Hits.test.tsx b/packages/instantsearch.js/src/components/Hits/__tests__/Hits.test.tsx
@@ -28,7 +28,7 @@ describe('Hits', () => {
 
     expect(props.sendEvent).toHaveBeenCalledTimes(1);
     expect(props.sendEvent).toHaveBeenLastCalledWith(
-      'click',
+      'click:internal',
       props.hits[0],
       'Hit Clicked'
     );

diff --git a/packages/instantsearch.js/src/components/Hits/__tests__/__snapshots__/Hits-test.tsx.snap b/packages/instantsearch.js/src/components/Hits/__tests__/__snapshots__/Hits-test.tsx.snap
@@ -3,7 +3,6 @@
 exports[`Hits markup should render <Hits /> 1`] = `
 <div
   className="root"
-  onClick={[Function]}
 >
   <ol
     className="list"
@@ -33,7 +32,6 @@ exports[`Hits markup should render <Hits /> 1`] = `
 exports[`Hits markup should render <Hits /> without highlight function 1`] = `
 <div
   className="root"
-  onClick={[Function]}
 >
   <ol
     className="list"

diff --git a/packages/instantsearch.js/src/components/InfiniteHits/InfiniteHits.tsx b/packages/instantsearch.js/src/components/InfiniteHits/InfiniteHits.tsx
@@ -71,7 +71,7 @@ const InfiniteHits = ({
   }
 
   return (
-    <div className={cssClasses.root} onClick={handleInsightsClick}>
+    <div className={cssClasses.root}>
       {hasShowPrevious && (
         <Template
           {...templateProps}
@@ -96,8 +96,9 @@ const InfiniteHits = ({
             rootTagName="li"
             rootProps={{
               className: cssClasses.item,
-              onClick: () => {
-                sendEvent('click', hit, 'Hit Clicked');
+              onClick: (event: MouseEvent) => {
+                handleInsightsClick(event);
+                sendEvent('click:internal', hit, 'Hit Clicked');
               },
             }}
             key={hit.objectID}

diff --git a/packages/instantsearch.js/src/components/InfiniteHits/__tests__/InfiniteHits.test.tsx b/packages/instantsearch.js/src/components/InfiniteHits/__tests__/InfiniteHits.test.tsx
@@ -28,7 +28,7 @@ describe('InfiniteHits', () => {
 
     expect(props.sendEvent).toHaveBeenCalledTimes(1);
     expect(props.sendEvent).toHaveBeenLastCalledWith(
-      'click',
+      'click:internal',
       props.hits[0],
       'Hit Clicked'
     );

diff --git a/packages/instantsearch.js/src/lib/insights/__tests__/insights-listener-test.tsx b/packages/instantsearch.js/src/lib/insights/__tests__/insights-listener-test.tsx
@@ -132,7 +132,7 @@ describe('createInsightsEventHandler', () => {
           instantSearchInstance: createInstantSearch(),
           methodName: 'bindEvent',
           args: ['click', { objectID: '1', __position: 1 }, 'Hit Clicked'],
-        })
+        }).payloads
       );
 
       const Hits = (props: InsightsEventHandlerOptions) => (
@@ -176,7 +176,7 @@ describe('createInsightsEventHandler', () => {
           instantSearchInstance: createInstantSearch(),
           methodName: 'bindEvent',
           args: ['click', { objectID: '1', __position: 1 }, 'Hit Clicked'],
-        })
+        }).payloads
       );
 
       const Hits = (props: InsightsEventHandlerOptions) => (
@@ -250,7 +250,7 @@ describe('createInsightsEventHandler', () => {
           instantSearchInstance: createInstantSearch(),
           methodName: 'bindEvent',
           args: ['click', { objectID: '1', __position: 1 }, 'Product Clicked'],
-        })
+        }).payloads
       );
 
       const Hits = (props: InsightsEventHandlerOptions) => (