Skip to content

feat: DH-22030: dx event handlers plan#1373

Open
jnumainville wants to merge 2 commits into
deephaven:mainfrom
jnumainville:22030_dx_event_plan
Open

feat: DH-22030: dx event handlers plan#1373
jnumainville wants to merge 2 commits into
deephaven:mainfrom
jnumainville:22030_dx_event_plan

Conversation

@jnumainville

Copy link
Copy Markdown
Collaborator

Adds plan for dx event handlers

@jnumainville jnumainville requested review from dsmmcken and mofojed June 22, 2026 20:56

`traceName` is the Plotly trace name, which for Deephaven charts is the `by` column value (or partition key) that identifies the trace. If the chart has no `by`/partitioning, `traceName` is the default Plotly trace name.

### Layout Events (`on_relayout`)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will these events fire for every data point that ticks in? Something to keep in mind.

@jnumainville jnumainville Jun 23, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one should not fire for every data point. The events that would have been filtered out already.
We only update a couple things, such as title and legend title, in the layout after init. So besides that case, this mainly updates for user initiated adjustment such as zooming. This is why we freeze the layout too, so we can have the events such as zoom.

Comment thread plans/DH-22030-plotly-events.md Outdated


fig = dx.sunburst(
pop_by_continent, names="Continent", values="Pop", on_sunburst_click=handle_sunburst

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's kind of annoying that we need to do on_sunburst_click for the plot vs. just doing a on_click - we already know it's a sunburst chart, why not just do an on_click handler when calling from dx.sunburst?
I guess in dx.layer we could have multiple types of charts, so I imagine we need to have more information on the click event there; so do we have one on_click and add different metadata for the type of click, or do we force the on_click, on_sunburst_click when we're in a layer (we'll still need extra metadata to link it to which plot was clicked either way yeah?)

@jnumainville jnumainville Jun 23, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have some generated plotly.js code I was playing with and after some further research I was missing something here. The plotly_click event fires already, even for the hierarchical plots, it just wasn't wired.
The hierarchical plots have different events because they actually change the visualized data, which is what I thought the key difference was, but the actual reason they exist is if you return False from the callback you can prevent the drill down.
I'm thinking that instead of that, we expose a drill_down param that can be used to disable drill down. Downside is it's not as flexible (no conditional disabling several layers down for example) but this is a helpful and simple switch for the basic use case where you just want the data out of the level you click on.
Also, we are filtering out most of the trace data. The idea is you can use the curve number to look up the specific trace if you need to. This has made me think it's worth piping through the trace type too.

Comment thread plans/DH-22030-plotly-events.md Outdated


def handle_legend_click(event):
print(f"Legend clicked: {event['traceName']} (curve {event['curveNumber']})")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm do we want these events converted to snake_case instead of camelCase...

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I'll do that


### `on_relayout` — Layout Change (Pan/Zoom)

**Justification:** Synchronize the visible time range across multiple charts (e.g. pan one chart, update all others to the same window) or filter tables to the visible range.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I accept this justification, as it was an event I questioned. Concerned it might fire more than expected.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment to Bender. This specific one should be safe.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a detailed implementation plan for DH-22030 to support server-side Python callbacks for Plotly Express (dx) chart interaction events, including payload schemas, API signatures, and a phased rollout across Python + JS.

Changes:

  • Specifies supported Plotly event → Python callback parameters (plus hierarchical drill_down behavior).
  • Documents event payload schemas and provides usage examples for each event type.
  • Outlines a phased implementation plan across DeephavenFigure, message handling, and frontend wiring (plus proposed test coverage).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.


def handle_relayout(event):
if "xaxis.range[0]" in event:
print(f"Visible range: {event['xaxis.range[0]']} to {event['xaxis.range[1]']}")
Comment on lines +479 to +481
## Phase 3.5 — Selection Modebar Buttons

**File:** `packages/chart/src/Chart.tsx` (in `web-client-ui`)
Comment on lines +450 to +452
def wrap_callable(fn: Callable) -> Callable:
sig = signature(fn)
max_args = 0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants