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

Spacing between bricks in a heatmap #858

Closed
CalvinFernandez opened this issue Aug 16, 2016 · 8 comments
Closed

Spacing between bricks in a heatmap #858

CalvinFernandez opened this issue Aug 16, 2016 · 8 comments
Labels
community community contribution feature something new

Comments

@CalvinFernandez
Copy link

image

I'm trying to add spaces between bricks in my plotly heatmap to make it look like the above image. The closest workaround I've found to solving this is to implement something like http://codepen.io/etpinard/pen/dMQrKN. Would it be possible to add a padding field to the heatmap api that will specify padding for bottom, left, top and right of each heatmap brick?

CalvinFernandez pushed a commit to CalvinFernandez/plotly.js that referenced this issue Aug 16, 2016
Add padding field to heatmap that allows users to define space
between bricks.
CalvinFernandez pushed a commit to CalvinFernandez/plotly.js that referenced this issue Aug 17, 2016
Add padding field to heatmap that allows users to define space
between bricks.
@CalvinFernandez
Copy link
Author

( @etpinard / not sure who to ping ) would someone mind taking a look here

master...CalvinFernandez:heatmap-padding

@etpinard etpinard added feature something new community community contribution labels Aug 17, 2016
@etpinard
Copy link
Contributor

@CalvinFernandez great initiative. We've been wanting to add this feature for a while now. 🍻 to that!

There a few things we should clear up before making your PR though.

  • Is padding, with l, t, r and b items really necessary? I'm thinking that two attributes: one controlling horizontal padding and another for vertical padding should be enough. Unless you can convince me otherwise.
  • a few other minor points that I'll make on your a508d28 commit

@CalvinFernandez
Copy link
Author

@etpinard 👍 to horizontal and vertical.

@etpinard
Copy link
Contributor

👍 to horizontal and vertical.

@CalvinFernandez Great. In this case, I would prefer a flat attribute set instead of padding container.

plotly.js has alrady a few gap attributes like bargap and boxgap So, I'd vote for hgap and vgap. But, that might not be verbose enough. @cldougl what do you think?

@CalvinFernandez
Copy link
Author

alternatively xgap ygap is terse and clear?

@etpinard
Copy link
Contributor

xgap ygap is terse and clear?

+1

@CalvinFernandez
Copy link
Author

hey, @etpinard I addressed the comments; mind taking another look?

@etpinard
Copy link
Contributor

@CalvinFernandez looks great! PR away 🚀

One thing though, when you'll make your PR, the image tests fail on CI because your newly added heatmap_brick_padding.json mock has no corresponding baseline image. If you're feeling adventurous, you can try generate that baseline image locally using our image-test docker container. See docs here. If you find that too much of a pain, I'll upload the CI artefact to the PR thread.

CalvinFernandez pushed a commit to CalvinFernandez/plotly.js that referenced this issue Aug 19, 2016
Add padding field to heatmap that allows users to define space
between bricks.
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

No branches or pull requests

2 participants