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.

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

feat(hits): implement getWidgetRenderState #4525

Merged
merged 1 commit into from
Oct 13, 2020

Conversation

yannickcr
Copy link
Contributor

This implements the getWidgetRenderState widget lifecycle hook in hits.

@yannickcr yannickcr requested a review from a team September 28, 2020 15:29
@ghost ghost requested review from eunjae-lee and francoischalifour and removed request for a team September 28, 2020 15:29
@codesandbox-ci
Copy link

codesandbox-ci bot commented Sep 28, 2020

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 2ac74b7:

Sandbox Source
InstantSearch.js Configuration

@eunjae-lee
Copy link
Contributor

Are you going to implement getRenderState in a separate PR?
If not, please check this for example: https://github.com/algolia/instantsearch.js/pull/4518/files#diff-87f24c931e875f242d763837383e9251R136
I've implemented both getRenderState and getWidgetRenderState in some connectors :)

@yannickcr
Copy link
Contributor Author

I don't know if we decided on this yet, but I was more thinking about doing getRenderState in another PR.

@yannickcr yannickcr changed the base branch from feat/render-state to feat/improve-panel September 30, 2020 09:28
@yannickcr yannickcr force-pushed the feat/getWidgetRenderState-hits branch from ec03d62 to dc478c6 Compare September 30, 2020 09:29
@yannickcr
Copy link
Contributor Author

As discussed together, I added the getRenderState method in this PR. I also changed the base branch to feat/improve-panel.

Copy link
Contributor

@Haroenv Haroenv left a comment

Choose a reason for hiding this comment

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

this code looks good to me 👍

@yannickcr yannickcr force-pushed the feat/getWidgetRenderState-hits branch from dc478c6 to 311654f Compare October 1, 2020 10:27
Base automatically changed from feat/improve-panel to feat/render-state October 9, 2020 12:24
@yannickcr yannickcr force-pushed the feat/getWidgetRenderState-hits branch 2 times, most recently from dc478c6 to bb6cbb0 Compare October 12, 2020 15:10
@yannickcr yannickcr force-pushed the feat/getWidgetRenderState-hits branch from bb6cbb0 to 2ac74b7 Compare October 12, 2020 15:19
Copy link
Contributor

@Haroenv Haroenv left a comment

Choose a reason for hiding this comment

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

still lgtm after rebase

@yannickcr yannickcr merged commit d0f96d7 into feat/render-state Oct 13, 2020
@yannickcr yannickcr deleted the feat/getWidgetRenderState-hits branch October 13, 2020 09:11
Haroenv pushed a commit that referenced this pull request Nov 30, 2020
Haroenv pushed a commit that referenced this pull request Nov 30, 2020
Haroenv added a commit that referenced this pull request Dec 9, 2020
* feat(core): introduce `getWidgetUiState` lifecycle hook (1/n) (#4454)

This deprecates `getWidgetState` to `getWidgetUiState` to make the upcoming `getWidgetRenderState` less confusing.

* feat(core): introduce `getWidgetRenderState` (2/n) (#4457)

This introduces the widget lifecycle hook `getWidgetRenderState` and implements it for `connectSearchBox`.

* feat(autocomplete): implement `getWidgetRenderState` (#4466)

* feat(breadcrumb): implement `getWidgetRenderState` (#4467)

* feat(clearRefinements): implement `getWidgetRenderState` (#4468)

* feat(configure): implement `getWidgetRenderState` (#4469)

* feat(currentRefinements): implement `getWidgetRenderState` (#4470)

* fix(breadcrumb): add attribute to render state (#4472)

* feat(hierarchicalMenu): implement `getWidgetRenderState` (#4471)

* fix: provide both `getWidgetRenderState` and `getRenderState` in connectors (#4518)

* WIP

* remove temporary implementation

* rename getWidgetRenderState to getRenderState

* fix wrong parameter name

* update panel

* update comment

* chore: remove panel-related code

* fixing types WIP

* fix type

* do not cast the return of getWidgetRenderState

* Revert "do not cast the return of getWidgetRenderState"

This reverts commit 614bc53.

* Revert "fix type"

This reverts commit 54e31fc.

* add TWidgetRenderState to connector

* add generics to Widget for getWidgetRenderState

* fix to allow nullish getWidgetRenderState when unknown

* remove exclamation marks

* remove the type for widgetParams (was experimental)

* make getRenderState optional for widgets with default generics

* update other connectors to follow new connector type

* update types in tests

* add comment

* do not declare individual widget render state types

* feat(hits): implement `getWidgetRenderState` (#4525)

* feat(range): implement `getRenderState` and `getWidgetRenderState` (#4536)

* fix(types): fix type errors (#4537)

* fix type errors

* fix type errors

* simplify test util

* update type of widgetParams

* add lifecycle methods

* fixing breadcrumb widget

* revert the change (let's do this in later iteration)

* add lifecycle methods for places widget

* replace WidgetFactory with Factory

* remove unnecessary part

* rename options to params

* Revert "remove unnecessary part"

This reverts commit 5b2ed14.

* Revert "replace WidgetFactory with Factory"

This reverts commit ee125bd.

* fix getRenderState of places and analytics

* feat(poweredBy): getWidgetRenderState (#4551)

* feat(poweredBy): getWidgetRenderState

DX-206

* fix jsdoc

* feat(menu): implement `getRenderState` and `getWidgetRenderState` (#4540)

* feat(menu): implement `getRenderState` and `getWidgetRenderState`

* Removed duplicated declaration of `jsHelper`, moved `cachedToggleShowMore` binding to `init` to avoid it happens more than once

* feat(renderState): add connectNumericMenu (#4550)

* feat(renderState): add connectNumericMenu

DX-204

* Apply suggestions from code review

Co-authored-by: Yannick Croissant <[email protected]>

* Apply suggestions from code review

Co-authored-by: Yannick Croissant <[email protected]>

* POC: Add telemetry

* Update src/widgets/configure-related-items/configure-related-items.ts

Co-authored-by: Haroen Viaene <[email protected]>

* Use full Schema

* POC: Add telemetry 2

* Use middleware

* move code a little

* chore: fix type of middleware

TODO: make sure this stays on rebase

Co-authored-by: François Chalifour <[email protected]>
Co-authored-by: Eunjae Lee <[email protected]>
Co-authored-by: Yannick Croissant <[email protected]>
Co-authored-by: Clément Vannicatte <[email protected]>
Co-authored-by: Yannick Croissant <[email protected]>
Haroenv added a commit that referenced this pull request Dec 18, 2020
* feat(core): introduce `getWidgetUiState` lifecycle hook (1/n) (#4454)

This deprecates `getWidgetState` to `getWidgetUiState` to make the upcoming `getWidgetRenderState` less confusing.

* feat(core): introduce `getWidgetRenderState` (2/n) (#4457)

This introduces the widget lifecycle hook `getWidgetRenderState` and implements it for `connectSearchBox`.

* feat(autocomplete): implement `getWidgetRenderState` (#4466)

* feat(breadcrumb): implement `getWidgetRenderState` (#4467)

* feat(clearRefinements): implement `getWidgetRenderState` (#4468)

* feat(configure): implement `getWidgetRenderState` (#4469)

* feat(currentRefinements): implement `getWidgetRenderState` (#4470)

* fix(breadcrumb): add attribute to render state (#4472)

* feat(hierarchicalMenu): implement `getWidgetRenderState` (#4471)

* fix: provide both `getWidgetRenderState` and `getRenderState` in connectors (#4518)

* WIP

* remove temporary implementation

* rename getWidgetRenderState to getRenderState

* fix wrong parameter name

* update panel

* update comment

* chore: remove panel-related code

* fixing types WIP

* fix type

* do not cast the return of getWidgetRenderState

* Revert "do not cast the return of getWidgetRenderState"

This reverts commit 614bc53.

* Revert "fix type"

This reverts commit 54e31fc.

* add TWidgetRenderState to connector

* add generics to Widget for getWidgetRenderState

* fix to allow nullish getWidgetRenderState when unknown

* remove exclamation marks

* remove the type for widgetParams (was experimental)

* make getRenderState optional for widgets with default generics

* update other connectors to follow new connector type

* update types in tests

* add comment

* do not declare individual widget render state types

* feat(hits): implement `getWidgetRenderState` (#4525)

* feat(range): implement `getRenderState` and `getWidgetRenderState` (#4536)

* fix(types): fix type errors (#4537)

* fix type errors

* fix type errors

* simplify test util

* update type of widgetParams

* add lifecycle methods

* fixing breadcrumb widget

* revert the change (let's do this in later iteration)

* add lifecycle methods for places widget

* replace WidgetFactory with Factory

* remove unnecessary part

* rename options to params

* Revert "remove unnecessary part"

This reverts commit 5b2ed14.

* Revert "replace WidgetFactory with Factory"

This reverts commit ee125bd.

* fix getRenderState of places and analytics

* feat(poweredBy): getWidgetRenderState (#4551)

* feat(poweredBy): getWidgetRenderState

DX-206

* fix jsdoc

* feat(menu): implement `getRenderState` and `getWidgetRenderState` (#4540)

* feat(menu): implement `getRenderState` and `getWidgetRenderState`

* Removed duplicated declaration of `jsHelper`, moved `cachedToggleShowMore` binding to `init` to avoid it happens more than once

* feat(renderState): add connectNumericMenu (#4550)

* feat(renderState): add connectNumericMenu

DX-204

* Apply suggestions from code review

Co-authored-by: Yannick Croissant <[email protected]>

* Apply suggestions from code review

Co-authored-by: Yannick Croissant <[email protected]>

* POC: Add telemetry

* Update src/widgets/configure-related-items/configure-related-items.ts

Co-authored-by: Haroen Viaene <[email protected]>

* Use full Schema

* POC: Add telemetry 2

* Use middleware

* move code a little

* chore: fix type of middleware

TODO: make sure this stays on rebase

Co-authored-by: François Chalifour <[email protected]>
Co-authored-by: Eunjae Lee <[email protected]>
Co-authored-by: Yannick Croissant <[email protected]>
Co-authored-by: Clément Vannicatte <[email protected]>
Co-authored-by: Yannick Croissant <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants