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

fix for figure resizing in webagg #3341

Closed
wants to merge 1 commit into from

Conversation

eyurtsev
Copy link
Contributor

@eyurtsev eyurtsev commented Aug 3, 2014

This is a fix for this issue: #3338

Currently, resizing does not work well in the webagg backend because mouse events are not ignored during the resize, which makes it both difficult and slow to do.

The code below ignores mouse events while resizing, but the fix is only for the figure in question. (So resizing figure 1 over figure 2 won't stop figure 2 from responding to mouse events.)

I also included some cosmetic changes: while resizing the figure, the figure is not rendered continuously but is replaced by a light gray box. This makes the resizing look a lot smoother in my opinion.

I have (very) little experience with javascript/html/css, so I wasn't sure where to place the CSS code for the "boxme" class. (It's directly in the javascript below, which probably isn't the best idea.)

@eyurtsev eyurtsev changed the title fixed figure resizing in webagg fix for figure resizing in webagg Aug 3, 2014
@tacaswell tacaswell added this to the v1.4.x milestone Aug 4, 2014
@pelson
Copy link
Member

pelson commented Aug 8, 2014

This looks good. Thanks @eyurtsev. It'd be good to pull the functionality out of the main figure function and into its own function. Also, when resizing animations, the animation frames are still updating the grayed out box - truthfully though, I'm not sure I'm 100% supportive of the graying out of the figure - that isn't the behaviour I get with other backends, and sometimes you want to resize the figure based on what the figure looks like (for instance, you might want to resize to optimize axes space for equal-aspect plots).

Also, did you test this with the nbagg backend?

@eyurtsev
Copy link
Contributor Author

Graying out the figure:

I agree I'd be happier if this could look exactly like the other
backends.

On my machine the plot wasn't updating quickly enough to render in real
time, so I found it more useful to work with a place holder for the figure
when resizing.

animations:
I have never used animations in matplotlib, so wouldn't even have known to check this.

nbagg:

The pull request is missing a var in front of the variable ctx.
ctx = fig.context; -> var ctx = fig.context;

Without this correction it fails in nbagg! (I wonder why it works with webagg)

I can submit a new pull request either implementing the issue with the variable or else entirely removing the graying out behavior. (But it'll be only in a few weeks.) I can also factor out the functionality out of the figure, but that can take sometime since I'll need to study the code more carefully. Let me know what you'd prefer.

@tacaswell tacaswell modified the milestones: v1.4.x, v1.4.1 Sep 3, 2014
@tacaswell
Copy link
Member

@eyurtsev Any interest in reviving this PR?

@pelson
Copy link
Member

pelson commented Jan 13, 2015

On my machine the plot wasn't updating quickly enough to render in real time, so I found it more useful to work with a place holder for the figure when resizing.

How about we limit the number of redraws when resizing - something like 1.5Hz would be a reasonable rate when resizing.

@eyurtsev
Copy link
Contributor Author

@tacaswell sorry just saw your message. i need to fix my github email preferences...

@eyurtsev
Copy link
Contributor Author

@pelson

I actually removed the debouncing in the new PR. It seems to be unnecessary now that the mouse events are ignored while resizing.

@tacaswell
Copy link
Member

Closing in favor of #4035.

@eyurtsev Please correct me if I am confused. In the future you can also use push/forcepush to update a PR with additional/brand new commits.

The PR tracks a merge from a fixed branch name, not from a fixed commit.

@tacaswell tacaswell closed this Jan 25, 2015
@eyurtsev eyurtsev deleted the fix_webagg_resize branch January 26, 2015 00:27
@eyurtsev
Copy link
Contributor Author

@tacaswell thanks for the information! I have only made a few PRs to date, and never revised one, so I wasn't sure how to do it.

Anyway, I applied the changes on top of the running master, so less changes to merge now. :)

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.

None yet

3 participants