Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 16 commits into from
Mar 16, 2023
Merged

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

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
16c096f
fix(insights): prevent duplicate events
Haroenv Mar 3, 2023
35028d0
fix(internal): bail out of default click if element has a bindEvent p…
Haroenv Mar 6, 2023
d78effe
test
Haroenv Mar 14, 2023
0fba3f7
fix: only prevent next clicks after a regular click
Haroenv Mar 14, 2023
54ae54f
bundlesize
Haroenv Mar 14, 2023
6ad9ebb
test(infiniteHits): add sendEvent tests
Haroenv Mar 14, 2023
44747c2
wip infinite hits
Haroenv Mar 14, 2023
d2671bb
move bindEvent click listener
Haroenv Mar 14, 2023
db6783e
refactor: move dedupe into middleware
Haroenv Mar 14, 2023
c5180bf
fix(events): move back inside connector
Haroenv Mar 15, 2023
bcd1155
get rid of global cruft
Haroenv Mar 15, 2023
81ceff4
refactor: use timers instead of order-based ignoring
Haroenv Mar 15, 2023
525e3ba
test(common): insights clicks + view for hits + infinitehits
Haroenv Mar 15, 2023
5b1d7ed
fix(react): send an internal event
Haroenv Mar 15, 2023
cf1ee9a
PR feedback
Haroenv Mar 15, 2023
1be6e7a
internal
Haroenv Mar 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions bundlesize.config.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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.50 kB"
},
{
"path": "packages/vue-instantsearch/vue2/cjs/index.js",
Expand Down
111 changes: 111 additions & 0 deletions packages/instantsearch.js/src/__tests__/common.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
createPaginationTests,
createMenuTests,
createInfiniteHitsTests,
createHitsTests,
} from '@instantsearch/tests';

import instantsearch from '../index.es';
Expand All @@ -19,6 +20,8 @@ import {
pagination,
infiniteHits,
searchBox,
hits,
index,
} from '../widgets';

createHierarchicalMenuTests(({ instantSearchOptions, widgetParams }) => {
Expand Down Expand Up @@ -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', () => {
/*
Expand Down
7 changes: 4 additions & 3 deletions packages/instantsearch.js/src/components/Hits/Hits.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯

sendEvent('click:internal', hit, 'Hit Clicked');
},
}}
key={hit.objectID}
Expand Down
2 changes: 1 addition & 1 deletion packages/instantsearch.js/src/components/Hits/__tests__/Hits.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ describe('Hits', () => {

expect(props.sendEvent).toHaveBeenCalledTimes(1);
expect(props.sendEvent).toHaveBeenLastCalledWith(
'click',
'click:internal',
props.hits[0],
'Hit Clicked'
);
Expand Down
2 changes: 0 additions & 2 deletions packages/instantsearch.js/src/components/Hits/__tests__/__snapshots__/Hits-test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
exports[`Hits markup should render <Hits /> 1`] = `
<div
className="root"
onClick={[Function]}
>
<ol
className="list"
Expand Down Expand Up @@ -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"
Expand Down
7 changes: 4 additions & 3 deletions packages/instantsearch.js/src/components/InfiniteHits/InfiniteHits.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ const InfiniteHits = ({
}

return (
<div className={cssClasses.root} onClick={handleInsightsClick}>
<div className={cssClasses.root}>
{hasShowPrevious && (
<Template
{...templateProps}
Expand All @@ -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}
Expand Down
2 changes: 1 addition & 1 deletion packages/instantsearch.js/src/components/InfiniteHits/__tests__/InfiniteHits.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ describe('InfiniteHits', () => {

expect(props.sendEvent).toHaveBeenCalledTimes(1);
expect(props.sendEvent).toHaveBeenLastCalledWith(
'click',
'click:internal',
props.hits[0],
'Hit Clicked'
);
Expand Down
6 changes: 3 additions & 3 deletions packages/instantsearch.js/src/lib/insights/__tests__/insights-listener-test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ describe('createInsightsEventHandler', () => {
instantSearchInstance: createInstantSearch(),
methodName: 'bindEvent',
args: ['click', { objectID: '1', __position: 1 }, 'Hit Clicked'],
})
}).payloads
);

const Hits = (props: InsightsEventHandlerOptions) => (
Expand Down Expand Up @@ -176,7 +176,7 @@ describe('createInsightsEventHandler', () => {
instantSearchInstance: createInstantSearch(),
methodName: 'bindEvent',
args: ['click', { objectID: '1', __position: 1 }, 'Hit Clicked'],
})
}).payloads
);

const Hits = (props: InsightsEventHandlerOptions) => (
Expand Down Expand Up @@ -250,7 +250,7 @@ describe('createInsightsEventHandler', () => {
instantSearchInstance: createInstantSearch(),
methodName: 'bindEvent',
args: ['click', { objectID: '1', __position: 1 }, 'Product Clicked'],
})
}).payloads
);

const Hits = (props: InsightsEventHandlerOptions) => (
Expand Down
Loading