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(poweredBy): getWidgetRenderState #4551

Merged
merged 2 commits into from
Oct 30, 2020
Merged

feat(poweredBy): getWidgetRenderState #4551

merged 2 commits into from
Oct 30, 2020

Conversation

Haroenv
Copy link
Contributor

@Haroenv Haroenv commented Oct 28, 2020

@codesandbox-ci
Copy link

codesandbox-ci bot commented Oct 28, 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 38dbec4:

Sandbox Source
InstantSearch.js Configuration

@eunjae-lee
Copy link
Contributor

It's too bad that it's not considered as "renamed and modified". Once this merged, will the file history of "connectPoweredBy.ts" start only from here?

@shortcuts
Copy link
Member

shortcuts commented Oct 28, 2020 •
edited
Loading

It's too bad that it's not considered as "renamed and modified". Once this merged, will the file history of "connectPoweredBy.ts" start only from here?

@eunjae-lee That's what I was wondering, I don't see any name changes on my side. The commit history will start from the new file.

EDIT: My bad, I didn't saw the .js > .ts

@eunjae-lee
Copy link
Contributor

Okay, I just did a test.
If you rename js to ts first, and commit them.
And in a separate commit, make actual changes there.
Then git will be able to track them correctly.

@Haroenv
Copy link
Contributor Author

Haroenv commented Oct 29, 2020

making a separate commit won't help here since we're in a batch PR which will be squashed at the end unfortunately @eunjae-lee

@eunjae-lee
Copy link
Contributor

making a separate commit won't help here since we're in a batch PR which will be squashed at the end unfortunately @eunjae-lee

ehh right.

@eunjae-lee
Copy link
Contributor

Then can you split the commits in this PR just to help us review the changes even though it will be squashed at the end?
It's quite difficult to figure out the changes @Haroenv

@Haroenv
Copy link
Contributor Author

Haroenv commented Oct 30, 2020

in 0435d68#diff-57893d658ec377d112dce456273d95a8dbdd6968bef10c1c65eb6d1b4272fcf1 you can see most of the changes to the connector, the test was only additive

eunjae-lee
eunjae-lee approved these changes Oct 30, 2020
View reviewed changes
Copy link
Contributor

@eunjae-lee eunjae-lee left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@Haroenv Haroenv merged commit bfd66e0 into feat/render-state Oct 30, 2020
@Haroenv Haroenv deleted the feat/render-state-poweredby branch October 30, 2020 10:28
Haroenv added a commit that referenced this pull request Nov 30, 2020
@Haroenv
feat(poweredBy): getWidgetRenderState (#4551)
33300c7 
* feat(poweredBy): getWidgetRenderState

DX-206

* fix jsdoc
Haroenv added a commit that referenced this pull request Nov 30, 2020
@Haroenv
feat(poweredBy): getWidgetRenderState (#4551)
cd816a4 
* feat(poweredBy): getWidgetRenderState

DX-206

* fix jsdoc
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

@eunjae-lee eunjae-lee eunjae-lee approved these changes

@shortcuts shortcuts Awaiting requested review from shortcuts

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Haroenv @eunjae-lee @shortcuts