feat(Insights): Insights inside Instantsearch #3598
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tkrugg
force-pushed
the
insights-connector
branch
2 times, most recently
from
ac1c448
to
765ac85
Compare
|
Deploy preview for instantsearchjs ready! Built with commit a7da8ed |
|
Deploy preview for instantsearchjs ready! Built with commit 77a7174 |
tkrugg
force-pushed
the
insights-connector
branch
2 times, most recently
from
a80fd78
to
97625ca
Compare
tkrugg
commented
Mar 12, 2019
tkrugg
commented
Mar 12, 2019
tkrugg
commented
Mar 12, 2019
tkrugg
force-pushed
the
insights-connector
branch
4 times, most recently
from
5fa170f
to
6ac560a
Compare
tkrugg
commented
Mar 18, 2019
samouss
requested changes
Mar 18, 2019
francoischalifour
requested changes
Mar 25, 2019
tkrugg
force-pushed
the
insights-connector
branch
from
54cc993
to
7b0595e
Compare
tkrugg
force-pushed
the
insights-connector
branch
4 times, most recently
from
d70b780
to
b183fb0
Compare
tkrugg
force-pushed
the
insights-connector
branch
from
89af5c0
to
b3e152c
Compare
tkrugg
force-pushed
the
insights-connector
branch
from
b3e152c
to
6b15b1c
Compare
samouss
reviewed
Mar 28, 2019
src/connectors/infinite-hits/__tests__/connectInfiniteHits-test.js
Outdated
Show resolved
Hide resolved
samouss
approved these changes
Mar 29, 2019
francoischalifour
requested changes
Mar 29, 2019
tkrugg
force-pushed
the
insights-connector
branch
from
b8e063b
to
3aa8b4e
Compare
francoischalifour
approved these changes
Apr 8, 2019
tkrugg
commented
Apr 10, 2019
|
@algolia/instantsearch-for-websites will squash this to ease the conflict solving. It shouldn't matter because we squash before merging anyway. |
tkrugg
force-pushed
the
insights-connector
branch
3 times, most recently
from
1146f41
to
b5a6850
Compare
feat(Insights): add withInsightsClient connector wrapper This PR adds a withInsightsClient which is an HOC for connectors. ```js const connectHitsWithInsightsClient = withInsightsClient(connectHits) ``` This connector will be used by default in the widget Hits and InfiniteHits to wrap connectHits and connectInfiniteHits respectively. When this PR is merged and released we can start using withInsightsClient in the flavours. feat(Insights): add template helpers feat(Insights): add withInsightsListener This commit adds withInsightsListener which - wraps Hits and InfiniteHits component, - listens to inner clicks targetting elements with data-insights attributes - calls the insights client exposed by `withInsightsClient` ```js const HitsWithInsightsListener = withInsightsListener(Hits) ``` feat(Insights): allow passing insightsClient to instantsearch instance feat(Insights): add listener to Hits feat(Insights): add listener to InfiniteHits feat(Insights): add storybook example for hits feat(Insights & typescript): type all the things extract *WithInsightsListeners components to upper scope rename withInsightsClient to withInsights feat(Insights): fix positions in infiniteScroll feat(Insights): extract addAbsolutePositions to make it connector responsibility unshorten variable name ev -> event rename isFirstRendering -> isFirstRender type mouse event avoid inline return clean unsused comment avoid inline return inline cast document.querySelector<HTMLElement> prettier things remove document maniputation in tests remove inferrable types in readDataAttributes use post modern typing type return values Apply suggestions from code review Co-Authored-By: tkrugg <tkrugg@users.noreply.github.com> add space before describe change inferPayload signature to use named params remove perf comment add space before Unmounter Apply suggestions from code review Co-Authored-By: tkrugg <tkrugg@users.noreply.github.com> Update src/lib/__tests__/insights-client-test.js Co-Authored-By: tkrugg <tkrugg@users.noreply.github.com> remove param mutation absolute imports before relative convert to typescript client and listener tests factorize Without inside 'types/utils' simplify types typo feedback: expose addAbsolutePosition in utils feedback: improved error message add type on serializedPayload Co-Authored-By: tkrugg <tkrugg@users.noreply.github.com> casting div.firstElementChild with as keyword Co-Authored-By: tkrugg <tkrugg@users.noreply.github.com> feedback: move casting on another line feedback: improve error message on incorrect data-insights-payload json string prettier all the things feedback: add type for Hits React component fix typescript file linting add query id feat(insights): add __queryID to connectHits & connectInfiniteHits simplified tests feat(insights): encode payload to base64 typo Co-Authored-By: tkrugg <tkrugg@users.noreply.github.com> typo Co-Authored-By: tkrugg <tkrugg@users.noreply.github.com> feedback: fix insightsClient defaulting to jest.fn() feedback: remove unused beforeEach feedback: group expect statements for payloads typo Co-Authored-By: tkrugg <tkrugg@users.noreply.github.com> feedback: add quotes around objectID Co-Authored-By: tkrugg <tkrugg@users.noreply.github.com> feedback: rename WidgetParams -> TWidgetParams feedback: move clickAnalytics: true to configure feedback: use toHaveBeenNthCalledWith feedback: remove extra check for insightClient callability feedback: remove proptypes fix: typo inline setup function fix: error message formatting Co-Authored-By: tkrugg <tkrugg@users.noreply.github.com> fix: error message formatting Co-Authored-By: tkrugg <tkrugg@users.noreply.github.com> fix: error message formatting Co-Authored-By: tkrugg <tkrugg@users.noreply.github.com> fix: error message formatting Co-Authored-By: tkrugg <tkrugg@users.noreply.github.com> fix: error message formatting Co-Authored-By: tkrugg <tkrugg@users.noreply.github.com> fix: error message formatting Co-Authored-By: tkrugg <tkrugg@users.noreply.github.com> fix: error message formatting Co-Authored-By: tkrugg <tkrugg@users.noreply.github.com> fix: error message formatting fix: error message formatting Co-Authored-By: tkrugg <tkrugg@users.noreply.github.com> fix: error message formatting Co-Authored-By: tkrugg <tkrugg@users.noreply.github.com> fix: error message formatting Co-Authored-By: tkrugg <tkrugg@users.noreply.github.com> fix: update snapshots after error message reformatting
tkrugg
force-pushed
the
insights-connector
branch
from
e6db95e
to
6244430
Compare
This is mimicking the way we allow usage for `highlight` and `snippet`.
```js
Instantsearch.widgets.hits({
// ...
templates: {
item(hit) {
return `
<h2> ${hit.name} </h2>
<button ${
Instantsearch.insights('clickedObjectIDsAfterSearch', {
eventName: "Add to favorite",
objectIDs: [hit.objectID]
})
}>
Add to favorite
</button>
`;
},
},
});
```
…sWithInsights The intention is to make it easier and more straight forward to create custom InfiniteHits and Hits with the connector. If we exposed `withInsights` directly, this is what we'd need to decide on a definitive name that implies it works only on Hits (like withHitsInsights) ```js const connectHitsWithInsights = Instantsearch.connectors.withHitsInsights(Instantsearch.connectors.connectHits); const connectInfiniteHitsWithInsights = Instantsearch.connectors.withHitsInsights(Instantsearch.connectors.connectInfiniteHits); ``` If we expose `connectHitsWithInsights` and `connectInfiniteHitsWithInsights` directly, we can keep `withInsights` totally private, and all custom components example are more simple.
tkrugg
force-pushed
the
insights-connector
branch
from
4f36403
to
b8d4c0c
Compare
tkrugg
added a commit
that referenced
this pull request
Apr 17, 2019
# [3.4.0](v3.3.0...v3.4.0) (2019-04-17) ### Bug Fixes * **storybook:** fix Hierarchical menu separator in Breadcrumb story ([#3695](#3695)) ([b3bf8ac](b3bf8ac)) * **tools:** use commonjs in bump-package-version.js ([#3699](#3699)) ([6a6dbe1](6a6dbe1)) * **types:** fix wrong typing in getWidgetState ([#3693](#3693)) ([b3c2154](b3c2154)) * **types:** remove unused Without type ([#3694](#3694)) ([656d000](656d000)) ### Features * **infiniteHits:** add previous button ([#3675](#3675)) ([2e6137b](2e6137b)) * **Insights:** Insights inside Instantsearch ([#3598](#3598)) ([387f41f](387f41f))
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR follows up on #3547 and aims to introduce 3 things
withInsightsClient: connector wrapper that exposes insights client to connectorshelpers/insights: which adds helpers that generatedata-insight-*attributeswithInsightsListener: which wraps Hits and InfiniteHits components and listens to clicks on elements containingdata-insights-*attributes.Result
WIP