Summary

This PR adds on_select support for hierarchical Plotly charts (treemap and sunburst) by wiring plotly_click into Streamlit's selection state. It introduces a new frontend utility (handleClickEvent), updates the Plotly component to register click handlers, and adds both unit and e2e coverage for treemap/sunburst selection behavior.

Code Quality

The implementation is generally clean and follows existing Plotly selection patterns, but there is one behavioral inconsistency that should be fixed before merge:

• Selection mode gating bug: In frontend/lib/src/components/elements/PlotlyChart/PlotlyChart.tsx (around lines 488-491), onClick is enabled whenever isSelectionActivated is true (any non-empty selection_mode). In frontend/lib/src/components/elements/PlotlyChart/utils.ts (around lines 347-352), handleClickEvent only checks for hierarchical point shape (id + parent) and does not verify that POINTS mode is enabled.
  This means charts configured with selection_mode=["box"] or selection_mode=["lasso"] can still emit click-based point selections on treemap/sunburst, which violates the expected semantics of selection_mode.

Test Coverage

Coverage is strong for the primary happy path:

• Added unit tests for handleClickEvent in frontend/lib/src/components/elements/PlotlyChart/utils.test.ts.
• Added e2e tests for treemap/sunburst click selection in e2e_playwright/st_plotly_chart_select_test.py.

However, an important negative case is missing:

• No test verifies that click-based hierarchical selection is ignored when POINTS mode is not selected.

Backwards Compatibility

Most changes are additive and preserve existing behavior for default on_select usage.
The exception is the selection-mode inconsistency above: users who intentionally disable point selection may now still get point-like reruns on hierarchical charts.

Security & Risk

No direct security concerns identified.
Main risk is behavioral regression in event semantics (selection_mode) that can cause unexpected reruns and callback triggers.

Accessibility

No accessibility regressions identified in this PR. The change is event wiring in an existing Plotly component and does not introduce new custom interactive DOM controls.

Recommendations

1. Gate hierarchical click handling on POINTS mode (e.g., attach onClick only when isPointsSelectionActivated, or early-return in handleClickEvent when element.selectionMode does not include POINTS).
2. Add a unit test in frontend/lib/src/components/elements/PlotlyChart/utils.test.ts asserting handleClickEvent does not call setStringValue when selectionMode excludes POINTS.
3. Add an e2e anti-regression case in e2e_playwright/st_plotly_chart_select_test.py for a hierarchical chart configured without "points" mode, asserting no selection output appears.

Verdict

CHANGES REQUESTED: The hierarchical click fix is directionally correct, but selection-mode semantics are currently violated for non-points configurations and should be corrected before merge.

This is an automated AI review by gpt-5.3-codex-high. Please verify the feedback and use your judgment.

Summary

This PR adds selection support for hierarchical Plotly chart types (treemap and sunburst) that don't emit native plotly_selected events but do emit plotly_click events. The implementation intercepts click events, detects hierarchical chart clicks by checking for id and parent properties on the clicked point, and routes them through the existing widget state mechanism as selection data. This is a targeted bugfix (closes #9001) with no changes to protobuf definitions or backend Python code.

Key changes:

• utils.ts: New handleClickEvent function that processes click events from hierarchical charts, formats the data consistently with the existing selection state schema, and avoids redundant state updates.
• PlotlyChart.tsx: Adds an onClick handler wired via useCallback, gated behind isSelectionActivated.
• utils.test.ts: Unit tests covering early returns, successful processing, undefined pointNumber, and deduplication.
• E2E tests: App script with treemap and sunburst charts, and a parameterized test that clicks on chart segments and verifies selection data appears.

Code Quality

The implementation follows existing patterns well. The handleClickEvent function mirrors the structure of handleSelection (guard clause, state construction, deduplication check before committing). The useCallback in the component follows the same eslint-disable pattern used by the neighboring handleSelectionCallback.

Minor observations:

1. utils.ts line 345 (const point = event.points[0] as any): The any cast is annotated with an eslint-disable comment, consistent with the existing codebase style. The PR only takes the first point from the click event. For treemap/sunburst, only one segment can be clicked at a time, so this is correct behavior.

2. utils.ts lines 349-352 — The guard clause point.id === undefined || point.parent === undefined is a heuristic to distinguish hierarchical chart clicks from regular chart clicks. This is a reasonable approach since id and parent are specific to hierarchical chart types in Plotly's event model. It would also correctly handle Plotly's icicle chart type (another hierarchical chart) — though this isn't documented or tested.

3. PlotlyChart.tsx line 491 — The onClick handler is attached to ALL charts when selection is activated, not just hierarchical ones. While the guard clause in handleClickEvent prevents side effects for non-hierarchical charts, this adds a minor overhead (function call + guard check) for every click on any Plotly chart with selection enabled. This is negligible and the alternative (detecting chart type at the component level) would be more complex.

4. PlotlyChart.tsx line 360 — The eslint-disable comment for react-hooks/exhaustive-deps uses element.id instead of element in the dependency array, matching the pattern of the neighboring handleSelectionCallback. The comment explaining why is missing for this callback (it exists on line 348-349 for handleSelectionCallback).

Test Coverage

Unit tests (utils.test.ts):

• Good use of it.each for parameterized early-return cases (undefined event, empty points, non-hierarchical click).
• Tests cover the core happy path (treemap/sunburst click with full data), edge case (undefined pointNumber), and deduplication (unchanged state).
• Tests follow the positive + negative assertion pattern: verifying setStringValue is NOT called for early returns and IS called for valid events.

E2E tests (st_plotly_chart_select_test.py):

• Tests use parameterized approach for treemap (index 7) and sunburst (index 8).
• The test verifies both the initial "no selection" state and the post-click selection state, which is good.
• Includes a negative assertion (not_to_be_attached for the "No selection" text).

Suggestions for improved coverage:

1. E2E locators: The test uses app.get_by_test_id("stPlotlyChart").nth(7) and .nth(8) — index-based locators are fragile. Since the charts have keys (treemap_chart, sunburst_chart), using get_element_by_key(app, "treemap_chart").get_by_test_id("stPlotlyChart") would be more robust and aligned with the E2E testing best practices documented in e2e_playwright/AGENTS.md.
2. Double-click reset: There's no E2E or unit test verifying that double-clicking a hierarchical chart properly resets the selection (via the existing onDoubleClick handler). This is a common user interaction that should be covered.
3. E2E text assertions: app.get_by_text("Selected:") and app.get_by_text("ID:") are generic and could match other elements on the page. More specific assertions like app.get_by_text(re.compile(r"Selected: .+")) or checking within a scoped container would be more robust.

Backwards Compatibility

This change is fully backwards compatible:

• No protobuf changes.
• No Python backend changes.
• The onClick handler is only active when isSelectionActivated is true (same gate as existing selection handlers).
• For non-hierarchical charts, the guard clause in handleClickEvent returns early without any side effects.
• The selection data format follows the existing PlotlyWidgetState schema.

Security & Risk

• Low risk: The change only processes client-side Plotly click events and routes them through the existing widget state mechanism. No new network calls, no new data sources, no user input parsing beyond what Plotly provides.
• Regression risk: The onClick handler is now attached to all Plotly charts with selection enabled. If a future Plotly version adds id/parent to non-hierarchical chart click events, it could cause unexpected selection behavior. However, this is unlikely and the guard clause provides a reasonable defense.
• No server-side changes: All changes are frontend-only, limiting the blast radius.

Accessibility

• No new interactive UI elements are added; the change leverages existing Plotly chart interaction patterns (clicking on chart segments).
• The selection behavior is consistent with the existing selection model, so assistive technology behavior should be unchanged.
• No accessibility concerns identified.

Recommendations

1. Use key-based locators in E2E tests: Replace app.get_by_test_id("stPlotlyChart").nth(7) with get_element_by_key(app, "treemap_chart").get_by_test_id("stPlotlyChart") (and similarly for sunburst). This follows the documented best practices and is more resilient to future changes in the test app.

2. Add a double-click reset test: Add an E2E test (or extend the parameterized test) to verify that double-clicking a hierarchical chart resets the selection, since the onDoubleClick handler is already wired up but not tested for this chart type.

3. Add a comment on the handleClickCallback dependency array: The neighboring handleSelectionCallback has a comment explaining why element.id is used instead of element. Add the same comment to handleClickCallback for consistency:

   // We are using element.id here instead of element since
   // shallow reference equality will not work correctly for element.

4. Consider documenting icicle chart support: The handleClickEvent function will also work for Plotly's icicle charts (another hierarchical type with id/parent properties). Consider mentioning this in the comments or adding an icicle test case to validate.

Verdict

APPROVED: A clean, well-structured bugfix that correctly adds selection support for hierarchical Plotly charts. The implementation follows existing patterns, includes good test coverage, and poses minimal regression risk. The recommendations above are minor improvements that could be addressed in a follow-up.

This is an automated AI review by opus-4.6-thinking.