Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add pattern fill for scatter filled area #6101

Merged
merged 5 commits into from
Feb 18, 2022

Conversation

s417-lama
Copy link
Contributor

Pattern fill for bar (and related trace types) has been added in #5520, but no pattern fill was available for scatter's filled area plots.

This PR adds pattern fill for scatter filled area.
I would like to add pattern fill for other trace types (box, violin, etc.) as well in the future, but this PR only includes scatter.

Sample file:
scatter_fill_pattern

(By the way, how can I run test-image locally? The workflow has been changed since I created my last PR, and how to run test-image seems not described in CONTRIBUTING.md.)

@archmoj archmoj added status: reviewable community community contribution feature something new labels Jan 30, 2022
@archmoj
Copy link
Contributor

archmoj commented Feb 7, 2022

Looking good now.
@alexcjohnson would you mind providing the second review?

Copy link
Collaborator

@alexcjohnson alexcjohnson left a comment

Choose a reason for hiding this comment

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

💃 Thanks @s417-lama - the one thing I was a little concerned about is removing fullLayout._*UrlQueryParts, just because I'm not really sure why that solution was added in the first place. But I think your class-based solution is better anyway, it's certainly simpler and clearer!

@archmoj archmoj merged commit 3aa9559 into plotly:master Feb 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community community contribution feature something new
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants