-
Notifications
You must be signed in to change notification settings - Fork 528
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(breadcrumb): implement getWidgetRenderState
#4467
feat(breadcrumb): implement getWidgetRenderState
#4467
Conversation
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 f1580a3:
|
@@ -29,7 +29,7 @@ storiesOf('Metadata/Breadcrumb', module) | |||
virtualHierarchicalMenu(), | |||
|
|||
breadcrumb({ | |||
container: breadcrumb, | |||
container: breadcrumbDiv, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This currently breaks in the prod deployment.
TWidgetParams | ||
> = TWidgetRenderState & { | ||
widgetParams: TWidgetParams; | ||
widgetParams: any; // @TODO type as TWidgetParams |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll take care of this in a future PR.
createURL: connectorState.createURL, | ||
items, | ||
refine: connectorState.refine, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
createURL & refine aren't cached like in clearRefinement, is that expected?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I kept it like it was in this PR, but it's not expected. There are multiple connectors that don't keep the same reference. We should fix this and automate some tests run on all widgets.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks equivalent to the previous code, still have a question though
* 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]>
* 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]>
This implements the
getWidgetRenderState
widget lifecycle hook inbreadcrumb
.