refactor: Move Panel into @deephaven/dashboard#2304
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2304 +/- ##
==========================================
- Coverage 47.31% 47.24% -0.07%
==========================================
Files 723 726 +3
Lines 39772 39785 +13
Branches 10144 9955 -189
==========================================
- Hits 18819 18798 -21
- Misses 20899 20976 +77
+ Partials 54 11 -43
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| @@ -0,0 +1,80 @@ | |||
| import React, { PureComponent, type ReactElement } from 'react'; | |||
| import { createXComponent } from '@deephaven/components'; | |||
| import { type BasePanelProps, Panel } from '@deephaven/dashboard'; | |||
There was a problem hiding this comment.
I feel like we should either have BasePanelProps, BasePanel or PanelProps, Panel for consistency
| export { default as CorePanel } from './CorePanel'; | ||
| // Deprecated - use CorePanel instead | ||
| export { default as Panel } from './CorePanel'; |
There was a problem hiding this comment.
Same comment about importing and renaming for the deprecated export w/ a deprecated tag.
CorePanel is what Panel used to be right? Why would someone need/want Panel from @deephaven/dashboard? dh.ui would still need the dashboard-core-components version, right? Or does nothing there need the session open/close events?
There was a problem hiding this comment.
deephaven.ui doesn't use the onSessionClosed or onSessionOpened props, it only needs the BasePanel
mattrunyon
left a comment
mattrunyon
left a comment
There was a problem hiding this comment.
I assume you've tested whatever we needed again since this is an old PR that looks like we forgot about. Looks like e2e and unit tests are passing after you updated from main.
I can't remember if we were holding off this PR for any reason or if it just fell off the radar.
- Move Panel into @deephaven/dashboard, removing functionality depending on core plugins and keeping in CorePanel in the dashboard-core-plugins package - Should be no breaking changes, as dashboard-core-plugins re-exports the components that were removed - Allows plugins (such as deephaven.ui) to simply use the `Panel` component without having to depend on other core plugins
- Was exporting it incorrectly, causing a breaking change. This way it appears correctly as "deprecated" instead of breaking
|
@mattrunyon I retested and rebased this. Found one issue with TabEvent deprecated export in dashboard-core-plugins, should be resolved now with no breaking changes. |
2 participants
Panelcomponent without having to depend on other core plugins