feat(panel): smoother accessing of widgetRenderState #4558
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Haroenv
commented
Oct 30, 2020
src/widgets/panel/panel.tsx
Outdated
| @@ -259,9 +259,8 @@ const panel: PanelWidget = widgetParams => { | |||
| const [renderOptions] = args; | |||
|
|
|||
| const options = { | |||
| ...(widget.getWidgetRenderState?.(renderOptions) || {}), | |||
There was a problem hiding this comment.
possible without casting since I reuse the type for TWidget for widgetFactory
Haroenv
commented
Oct 30, 2020
| collapsed({ widgetRenderState }) { | ||
| return widgetRenderState.canRefine; | ||
| collapsed({ canRefine }) { | ||
| return canRefine === false; |
There was a problem hiding this comment.
your example used to be wrong
|
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 9a8b4db:
|
| @@ -151,11 +151,13 @@ storiesOf('Basics/Panel', module) | |||
| withHits( | |||
| ({ search, container }) => { | |||
| const breadcrumbInPanel = panel<typeof breadcrumb>({ | |||
| collapsed({ widgetRenderState }) { | |||
| return widgetRenderState.canRefine; | |||
| collapsed({ canRefine }) { | |||
There was a problem hiding this comment.
This usage is much better and eventually what we want to enable in next major version so if there can't be collision with the IS instance (which is also passed and spread) that's great!
There was a problem hiding this comment.
Yeah, I didn't realize there cannot be collision. This looks so much better.
There was a problem hiding this comment.
as far as I can tell there's no collision here, so this doesn't even need a major :)
There was a problem hiding this comment.
Yeah, it makes sense there is no collision :)
eunjae-lee
approved these changes
Nov 2, 2020
| }; | ||
| > = RenderOptions & ReturnType<TWidget>['getWidgetRenderState'] extends ( | ||
| renderOptions: any | ||
| ) => infer TRenderState |
There was a problem hiding this comment.
didn't know about infer. looks nice 👍
eunjae-lee
pushed a commit
that referenced
this pull request
Nov 3, 2020
* update panel * add a story * provide an empty object to widgetRenderState by default * add generic to panel * update the story to infer the type instead of importing and using the type itself * extract to PanelRenderOptions * clean the code * fix type error * feat(panel): smoother accessing of widgetRenderState (#4558) * WIP * now it works * better example * revert for lower ts used here * gotta be more generic * WIP trying to replace any with unknown * add UnknownWidgetFactory * parenthesis, move back to any (unknown is stricter than any) * reuse UnknownWidgetFactory in panel component Co-authored-by: Haroen Viaene <[email protected]>
Haroenv
added a commit
that referenced
this pull request
Nov 30, 2020
* update panel * add a story * provide an empty object to widgetRenderState by default * add generic to panel * update the story to infer the type instead of importing and using the type itself * extract to PanelRenderOptions * clean the code * fix type error * feat(panel): smoother accessing of widgetRenderState (#4558) * WIP * now it works * better example * revert for lower ts used here * gotta be more generic * WIP trying to replace any with unknown * add UnknownWidgetFactory * parenthesis, move back to any (unknown is stricter than any) * reuse UnknownWidgetFactory in panel component Co-authored-by: Haroen Viaene <[email protected]>
Haroenv
added a commit
that referenced
this pull request
Nov 30, 2020
* update panel * add a story * provide an empty object to widgetRenderState by default * add generic to panel * update the story to infer the type instead of importing and using the type itself * extract to PanelRenderOptions * clean the code * fix type error * feat(panel): smoother accessing of widgetRenderState (#4558) * WIP * now it works * better example * revert for lower ts used here * gotta be more generic * WIP trying to replace any with unknown * add UnknownWidgetFactory * parenthesis, move back to any (unknown is stricter than any) * reuse UnknownWidgetFactory in panel component Co-authored-by: Haroen Viaene <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
this kinda works @eunjae-lee, although some parts still seem to be inferred as any. The public API works though
I also made a small change: not using widgetRenderState as key, but spreading the renderer. This way the usage is so much smoother (and you can use it for the templates too, which comes up from time to time)