Repository navigation
Bugfix/fillgradient bugs - #8096
Open
dewi-ny-je wants to merge 3 commits into
Open
dewi-ny-je wants to merge 3 commits into
dewi-ny-je wants to merge 3 commits into
Conversation
A fillgradient with start or stop uses bounds in user space. Four problems affected these bounds: - gradientWithBounds set them only when it created the gradient element, so the gradient did not follow a zoom or a pan - a missing bound read trace._extremes.x or .y, which does not exist for a trace on a secondary axis - the extremes are in linear space, but the code converted them with c2p, which is wrong on a log axis - a radial gradient with start or stop threw an error Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M7Q9TzC2e8vaoBxrhzAjhd
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M7Q9TzC2e8vaoBxrhzAjhd
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M7Q9TzC2e8vaoBxrhzAjhd
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #8095
Fixes four
fillgradientproblems inscattertraces. The work on #8093 found them. No upstream issue covers them yet.Before: A
fillgradientwithstartorstopkeeps its old bounds after a zoom or a pan. A singlestartorstopthrowsCannot read properties of undefined (reading 'max')on a secondary axis, and gives a wrong bound on a log axis. A radialfillgradientwithstartorstopthrowsCannot read properties of undefined (reading 'x').After: The gradient follows zoom and pan. A missing bound takes the lowest or highest value of the trace on its own axis, on linear and log axes. A radial gradient ignores
startandstop, as the attribute descriptions state.How: A new function in
src/components/drawing/index.js,axisGradient, computes the user-space bounds. It readstrace._extremesby axis id and converts the extremes withl2p, because they are in linear space.gradientWithBoundsnow sets the bounds on every call, not only when it creates the<linearGradient>.setFillStyleskips the user-space branch for radial.Tests
scatter_test, new blockscatter gradients: zoom, secondary axis, log axis, and radial. I did not run karma. I ran the four test bodies in headless Chromium againstbuild/plotly.jswith a small jasmine shim. All four pass with the fix and fail onmain.scatter_fill_gradient_tonextandscatter_fill_gradient_tonexty_toselfrender the same plot pixels before and after the change.npm run lint,npm run typecheck,npm run test-syntaxandnpm run schema-typegen-diff-checkpass.Draftlog
draftlogs/8096_fix.mdcarries the upstream PR number 8096.