Skip to content

fix(plotly): extract z-bbox from point.bbox.z, not point.bbox.y#6759

Open
anxkhn wants to merge 2 commits into
reflex-dev:mainfrom
anxkhn:patch-14
Open

fix(plotly): extract z-bbox from point.bbox.z, not point.bbox.y#6759
anxkhn wants to merge 2 commits into
reflex-dev:mainfrom
anxkhn:patch-14

Conversation

@anxkhn

@anxkhn anxkhn commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

All Submissions:

  • Have you followed the guidelines stated in CONTRIBUTING.md file?
  • Have you checked to ensure there aren't any other open Pull Requests for the desired changed?

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Changes To Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Have you successfully ran tests with your changes locally?

The client-side extractPoints helper injected by rx.plotly
(packages/reflex-components-plotly/src/reflex_components_plotly/plotly.py, in
add_custom_code()) reshapes each plotly.js hover/click point into reflex's
Point/BBox shape before it is sent to the Python on_hover/on_click/on_unhover
event handlers. When it builds the bounding box, the z0/z1 members were copied from
the y fields:

z0: point.bbox.y0,
z1: point.bbox.y1,

so for 3D traces (scatter3d/mesh3d/surface) the reported bbox.z0/bbox.z1
always duplicated the y-extent, and the real z bounding box was never surfaced. This is a
copy-paste typo: the two lines are identical to the y0/y1 lines above them except for
the destination key. reflex's own contract confirms the intent, the BBox TypedDict
declares z0/z1 as fields distinct from y0/y1, and Point has a z field.

The fix sources them from the matching z fields:

z0: point.bbox.z0,
z1: point.bbox.z1,

2D traces are unaffected: their point.bbox has no z fields, so removeUndefined drops
the extra keys either way.

Testing

Added test_plotly_extract_points_bbox_z_source in
tests/units/components/graphing/test_plotly.py, which renders the component, joins the
generated add_custom_code(), and asserts the snippet reads z0: point.bbox.z0 /
z1: point.bbox.z1 (and no longer z0: point.bbox.y0 / z1: point.bbox.y1). It fails
before the fix and passes after.

uv run pytest tests/units/components/graphing/test_plotly.py -q   # 5 passed
uv run ruff check .                                               # clean
uv run ruff format --check .                                      # clean
uv run pyright reflex tests                                       # clean (changed files)

A towncrier changelog fragment is included under
packages/reflex-components-plotly/news/.

anxkhn added 2 commits July 14, 2026 12:53
The injected extractPoints helper built the bbox z-extent from
point.bbox.y0/y1 instead of point.bbox.z0/z1, a copy-paste of the two
lines above it. As a result, on_hover/on_click/on_unhover event data for
3D traces reported a z0/z1 that always duplicated the y-extent, and the
real z bounding box was unreachable. The BBox TypedDict already declares
z0/z1 as fields distinct from y0/y1, so this only corrects the generated
JavaScript. 2D traces are unaffected: their point.bbox carries no z
fields, so the keys are still dropped by removeUndefined.

Add a regression test asserting the generated helper sources z0/z1 from
point.bbox.z0/point.bbox.z1.

Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
@anxkhn anxkhn requested a review from a team as a code owner July 14, 2026 07:23
@greptile-apps

greptile-apps Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR changes how Plotly event bounding boxes obtain their z fields. The main changes are:

  • Read z0 and z1 from matching point.bbox properties.
  • Add generated-code assertions for the new property access.
  • Add a bug-fix changelog entry.

Confidence Score: 4/5

The gl3d event extraction path still cannot return z bounding-box values.

  • Plotly gl3d event bounding boxes do not contain the properties read by the changed lines.
  • removeUndefined deletes the resulting values.
  • The new test checks generated text rather than the resulting event data.

packages/reflex-components-plotly/src/reflex_components_plotly/plotly.py and tests/units/components/graphing/test_plotly.py

Important Files Changed

Filename Overview
packages/reflex-components-plotly/src/reflex_components_plotly/plotly.py Reads z bounds from properties that are absent from Plotly gl3d event bounding boxes, so the values are removed as undefined.
tests/units/components/graphing/test_plotly.py Adds source-text assertions but does not exercise extraction with a representative runtime event payload.
packages/reflex-components-plotly/news/6722.bugfix.md Documents z-extent reporting that the updated extractor does not produce for gl3d events.

Reviews (1): Last reviewed commit: "chore(plotly): add changelog fragment fo..." | Re-trigger Greptile

Comment on lines 216 to +217
y1: point.bbox.y1,
z0: point.bbox.y0,
z1: point.bbox.y1,
z0: point.bbox.z0,

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.

P1 Z Bounds Remain Missing

Plotly gl3d events provide bbox as a screen-space box with x0, x1, y0, and y1, not z0 or z1. For scatter3d, mesh3d, and surface events, these reads therefore return undefined, and removeUndefined deletes both keys instead of exposing the claimed z extent.

@codspeed-hq

codspeed-hq Bot commented Jul 14, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 26 untouched benchmarks
⏩ 8 skipped benchmarks1


Comparing anxkhn:patch-14 (d63144a) with main (32cc257)

Open in CodSpeed

Footnotes

  1. 8 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

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.

1 participant