-
Notifications
You must be signed in to change notification settings - Fork 386
fix(MultiIndex): Trigger new search when <Index>
props are updated
#318
Conversation
Deploy preview ready! Built with commit 99bea20 |
<Index>
props are updated
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 looks good, nice feature
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.
LGTM I read it all
this.unregisterWidget = widgetsManager.registerWidget({ | ||
getSearchParameters: searchParameters => | ||
this.getSearchParameters(searchParameters, this.props), | ||
multiIndexContext: { |
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.
Could we also have solved it by making multiIndexContext a function rather than a statically computed object?
Just being curious
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.
What this function would have return?
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.
The updated indexName rather than the statically computed one. But your solution works and is more obvious: when registering a widget, you pass it as a whole
See #310
Basically until now you couldn't have dynamic
<Index>
.I think I also fix a bug with MultiIndex and SSR (a way to express multi index wasn't taken into account) with this PR.
What are the big changes here: the widget manager store the whole component instead of explicitly choose at constructor time.