refactor: Move Panel into @deephaven/dashboard #2304
Conversation
- 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
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2304 +/- ##
==========================================
- Coverage 46.92% 46.85% -0.08%
==========================================
Files 712 714 +2
Lines 39347 39359 +12
Branches 9840 9840
==========================================
- Hits 18463 18441 -22
- Misses 20873 20907 +34
Partials 11 11
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
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.
Panelcomponent without having to depend on other core plugins