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

Jump to bottom

feat(clearRefinements): implement getWidgetRenderState #4468

Merged
merged 2 commits into from
Aug 6, 2020
Merged

feat(clearRefinements): implement getWidgetRenderState #4468

merged 2 commits into from
Aug 6, 2020

Conversation

francoischalifour
Copy link
Member

This implements the getWidgetRenderState widget lifecycle hook in clearRefinements.

@codesandbox-ci
Copy link

codesandbox-ci bot commented Aug 4, 2020 •
edited
Loading

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 5f083d7:

Sandbox Source
InstantSearch.js Configuration

Haroenv
Haroenv reviewed Aug 5, 2020
View reviewed changes
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.

would like to see more code from render move to the life cycle, good otherwise

src/connectors/clear-refinements/connectClearRefinements.ts Outdated
@@ -154,6 +157,33 @@ const connectClearRefinements: ClearRefinementsConnector = function connectClear
dispose() {
unmountFn();
},

getWidgetRenderState(renderState, { scopedResults }) {
const attributesToClear = scopedResults.reduce<
Copy link
Contributor

Choose a reason for hiding this comment

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

this gets calculated both here and in refine itself still. Should we call it in the beginning of refine and use its value, instead of computing it again instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

Addressed in bfbe04d.

Haroenv
Haroenv requested changes Aug 5, 2020
View reviewed changes
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.

(left comment instead of requesting changes earlier by accident)

@francoischalifour francoischalifour force-pushed the feat/getWidgetRenderState-clearRefinements branch from bfbe04d to 5f083d7 Compare August 5, 2020 13:58
@francoischalifour francoischalifour merged commit b57606f into feat/render-state Aug 6, 2020
@francoischalifour francoischalifour deleted the feat/getWidgetRenderState-clearRefinements branch August 6, 2020 08:35
Haroenv pushed a commit that referenced this pull request Nov 30, 2020
@francoischalifour @Haroenv
feat(clearRefinements): implement getWidgetRenderState (#4468)
992072b
Haroenv pushed a commit that referenced this pull request Nov 30, 2020
@francoischalifour @Haroenv
feat(clearRefinements): implement getWidgetRenderState (#4468)
2b3117c
Haroenv added a commit that referenced this pull request Dec 9, 2020
@Haroenv @francoischalifour
@shortcuts @yannickcr
Poc/telemetry3 (#4601)
70ea711 
* 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
@Haroenv @francoischalifour
@shortcuts @yannickcr
Poc/telemetry3 (#4601)
3c06678 
* 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
Reviewers

@Haroenv Haroenv Haroenv approved these changes

@yannickcr yannickcr Awaiting requested review from yannickcr

Assignees
No one assigned
Labels
Scope: Render State
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@francoischalifour @Haroenv