fix: DH-18443: Fix dx and dh.ui tooltips#1226
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR enhances panel tooltip functionality by adding tooltips to dh.ui panels and externalizing dashboard core plugins for dx tooltips. The changes ensure consistent tooltip behavior across different panel types in the Deephaven dashboard.
- Adds tooltip support to
dh.uipanels with variable name display - Extracts variable name handling in ReactPanel for better tooltip integration
- Externalizes
@deephaven/dashboard-core-pluginsin plotly-express build configuration
Reviewed Changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| ReactPanel.tsx | Separates variable name extraction and passes it to layout configuration for tooltip support |
| PortalPanelTooltip.tsx | New component that renders tooltips for portal panels using WidgetPanelTooltip |
| PortalPanelTooltip.test.tsx | Unit tests for the new PortalPanelTooltip component |
| PortalPanel.tsx | Integrates tooltip rendering functionality into the Panel component |
| vite.config.ts | Adds dashboard-core-plugins to external dependencies for plotly-express |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
dx and dh.ui panelsdx and dh.ui tooltips
|
plotly-express docs preview (Available for 14 days) |
|
ui docs preview (Available for 14 days) |
|
plotly-express docs preview (Available for 14 days) |
|
ui docs preview (Available for 14 days) |
| glEventHub={glEventHub} | ||
| renderTabTooltip={() => ( | ||
| <PortalPanelTooltip | ||
| name={glContainer.getConfig().variableName} |
There was a problem hiding this comment.
Can we not just pull from metadata.name here?
| @@ -177,6 +178,7 @@ function ReactPanel({ | |||
| props: {}, | |||
There was a problem hiding this comment.
I think we just need to pass through the metadata as a prop here, no?
There was a problem hiding this comment.
Yep, way simpler, thanks
|
ui docs preview (Available for 14 days) |
|
plotly-express docs preview (Available for 14 days) |
|
plotly-express docs preview (Available for 14 days) |
|
ui docs preview (Available for 14 days) |
|
plotly-express docs preview (Available for 14 days) |
|
ui docs preview (Available for 14 days) |
There was a problem hiding this comment.
There shouldn't be any changes to the package-lock.json. Revert the diff on this file.
|
plotly-express docs preview (Available for 14 days) |
|
ui docs preview (Available for 14 days) |
|
plotly-express docs preview (Available for 14 days) |
|
ui docs preview (Available for 14 days) |
Externalizes
@deephaven/dashboard-core-pluginsfordxtooltipsAdds tooltip to
dh.ui