feat: DH-14581 usePanelRegistration hook#1208
Conversation
bmingles
force-pushed
the
1207-use-panel-registration
branch
from
26deadb to
751e335
Compare
Codecov Report
@@ Coverage Diff @@
## main #1208 +/- ##
==========================================
+ Coverage 44.20% 44.21% +0.01%
==========================================
Files 448 449 +1
Lines 33437 33445 +8
Branches 8404 8406 +2
==========================================
+ Hits 14781 14789 +8
Misses 18606 18606
Partials 50 50
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
mofojed
left a comment
mofojed
left a comment
There was a problem hiding this comment.
Just a minor suggestion for using null coalescing, but looks great!
| const name = | ||
| 'COMPONENT' in ComponentType | ||
| ? ComponentType.COMPONENT | ||
| : ComponentType.displayName; |
There was a problem hiding this comment.
I think you can just use null coalescing to handle this:
| const name = | |
| 'COMPONENT' in ComponentType | |
| ? ComponentType.COMPONENT | |
| : ComponentType.displayName; | |
| const name = ComponentType.COMPONENT ?? ComponentType.displayName; |
There was a problem hiding this comment.
I actually did that originally, but the type system complains when you access ComponentType.COMPONENT without verifying it's there
There was a problem hiding this comment.
nvm, fixed it in the types
| /** | ||
| * Registers a given panel component. Also runs a `useEffect` that will | ||
| * automatically de-register then panel on unmount. | ||
| * @param registerComponent |
There was a problem hiding this comment.
We might not want this as part of the hook. Bender and I chatted about plugins the other day and there's a lot of rough edges and things that need to be cleaned up around their implementation. Basically we should be able to write our existing plugins all as external plugins in an ideal world
hydrate and dehydrate are the same. IMO they are logic that belong in the component, not at the registration point. Our existing hydration code is in AppMainContainer which separates it from where the hydration is actually used.
I don't think they're changes that should be in this PR, but just something to note that this will probably change signatures as we address those points
mofojed
left a comment
mofojed
left a comment
There was a problem hiding this comment.
@mattrunyon hydration has to be available at the app level, but we should be moving widget panels to just use the default hydration when possible. metadata for data about the widget, fetch to get the object, panelState for storing any previous state set by the user.
|
|
||
| export type PanelFunctionComponentType<P> = NamedExoticComponent<P> & { | ||
| COMPONENT?: string; | ||
| TITLE?: string; |
There was a problem hiding this comment.
Since this is used here and in PanelComponentType, may want to separate it out and add some JSDocs about what each is for
bmingles
changed the title
3 participants
Supports Enterprise DH-14581
resolves #1207