Skip to content

Plot Area: Plot clip bug #490

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

Merged
merged 3 commits into from
Apr 28, 2016
Merged

Plot Area: Plot clip bug #490

merged 3 commits into from
Apr 28, 2016

Conversation

mdtusz
Copy link
Contributor

@mdtusz mdtusz commented Apr 28, 2016

Fixes #488. The d3 selection was only being created. This has added an update "join" on
the selection so the clips will be updated whenever relayout is called.

mdtusz added 2 commits April 28, 2016 11:43
The d3 selection was only being created. This has added an update "join" on
the selection.
@mdtusz mdtusz added bug something broken status: reviewable labels Apr 28, 2016
.append('rect')
.append('rect');

plotClip.selectAll('rect')
Copy link
Contributor

Choose a reason for hiding this comment

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

Nicely done. Man, I gotta got better at noticing these as a reviewer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I gotta get better at noticing these as a programmer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But really, I think it's just a symptom of the "d3 pattern". It's not explicitly clear and pretty tough to reason about what exactly each selection/block is doing.

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like d3 v4 will make this easier.

Taken from https://medium.com/@mbostock/what-makes-software-good-943557f8a488#.rb39u8abk

image

@etpinard
Copy link
Contributor

💃 after adding that one Plots.resize test.

@mdtusz mdtusz merged commit 4a9243e into master Apr 28, 2016
@mdtusz mdtusz deleted the plot-clip-bug branch April 28, 2016 18:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something broken
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants