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 Support for scroll_event in WebAgg and NbAgg [backport to 1.4.x] #3968

Merged
merged 22 commits into from Jan 11, 2015

Conversation

blink1073
Copy link
Member

Attached a scroll event handler to canvas_div and added the appropriate handling to backend_webagg_core.

@blink1073
Copy link
Member Author

Wow, that felt like a battle with Travis with all the guess-and-checking.

@tacaswell tacaswell added this to the v1.4.3 milestone Jan 5, 2015
@@ -402,7 +419,8 @@ mpl.figure.prototype.mouse_event = function(event, name) {
var x = canvas_pos.x;
var y = canvas_pos.y;

this.send_message(name, {x: x, y: y, button: event.button});
this.send_message(name, {x: x, y: y, button: event.button,
step: event.step});
Copy link
Member

Choose a reason for hiding this comment

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

what done js do if you have not set a value for event.step? I assume magically pass a null-like value...

Copy link
Member Author

Choose a reason for hiding this comment

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

JS assigns it to undefined which is translated to None AFAIK.

@tacaswell tacaswell changed the title Add Support for scroll_event in WebAgg and NbAgg Add Support for scroll_event in WebAgg and NbAgg [backport to 1.4.x] Jan 5, 2015
mouse_event_fn(event);
}

$(canvas_div)[0].addEventListener('DOMMouseScroll', scroll_fn, false);
Copy link
Member

Choose a reason for hiding this comment

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

Can you just do canvas_div.addEventListener(...) here? We have (what I think) is a reference to the js version of the div so there is no need to look it up.

Copy link
Member Author

Choose a reason for hiding this comment

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

Simplified now, does not work without the [0] access though.

Copy link
Member

Choose a reason for hiding this comment

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

Then why do the next two lines work?

Copy link
Member

Choose a reason for hiding this comment

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

That wasn't meant to be hostile, I am in fact quite confused.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm as confused as you are.

Copy link
Member Author

Choose a reason for hiding this comment

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

All I know is that canvas_div has no attribute addEventListener, but canvas_div[0] does.

@pelson
Copy link
Member

pelson commented Jan 5, 2015

Nice. 👍

@tacaswell
Copy link
Member

This now has conflicts with the enter/leave event PR.

@blink1073
Copy link
Member Author

Fixed (I hope).

@blink1073
Copy link
Member Author

The current code works in Firefox, Chrome, and IE. Turns out we did not need "mousewheel".

@tacaswell
Copy link
Member

Do we have any brave mac users who want to help out with safari?

On Tue Jan 06 2015 at 9:11:20 PM Steven Silvester notifications@github.com
wrote:

The current code works in Firefox, Chrome, and IE. Turns out we did not
need "mousewheel".


Reply to this email directly or view it on GitHub
#3968 (comment)
.

@tonysyu
Copy link
Contributor

tonysyu commented Jan 7, 2015

@blink1073: How are you testing this? (I'm not talking about real tests, just a manual verification that things are working)

@blink1073
Copy link
Member Author

@tonysyu, start ipython and %paste this:

import matplotlib
import itertools
import numpy as np
matplotlib.use('webagg')
import matplotlib.pyplot as plt
plt.close('all')
fig, ax = plt.subplots()
x = np.linspace(0,10,100)
y = np.sin(x)
ln, = ax.plot(x,y, 'o', picker=5)
evt = []
colors = iter(itertools.cycle(['r', 'g', 'b', 'k', 'c']))
def on_event(event):
    evt.append(event)
    ln.set_color(next(colors))
    fig.canvas.draw()
    fig.canvas.draw_idle()
fig.canvas.mpl_connect('scroll_event', on_event)
plt.show()

Then click on the figure and scroll and make sure it changes colors.

@tonysyu
Copy link
Contributor

tonysyu commented Jan 7, 2015

Does the suggestion to start ipython and paste actually work on your system? IPython should choose a backend at start-up so the matplotlib.use call shouldn't have an effect. At least it doesn't on my system.

In any case, when run outside of IPython, the script works for me on Safari and Chrome on OSX. 👍

It'd be nice to have an example that can be fired up from the examples directory that tests some of these events that are difficult to test properly. I'm not necessarily suggesting it for this PR, just throwing it out there.

@blink1073
Copy link
Member Author

@tonysyu, thanks! (I forgot to mention I had to comment out the matplotlib import in my ipython startup file). I agree about the demos.

@tacaswell
Copy link
Member

@honesty I think that the nbagg_uat.nb is that demo (which does not yet
have these tests in it)

On Tue, Jan 6, 2015, 23:21 Steven Silvester notifications@github.com
wrote:

@tonysyu https://github.com/tonysyu, thanks! (I forgot to mention I had
to comment out the matplotlib import in my ipython startup file). I agree
about the demos.


Reply to this email directly or view it on GitHub
#3968 (comment)
.

@tacaswell
Copy link
Member

@tonysyu darn you auto correct

On Wed, Jan 7, 2015, 01:07 Thomas Caswell tcaswell@gmail.com wrote:

@honesty I think that the nbagg_uat.nb is that demo (which does not yet
have these tests in it)

On Tue, Jan 6, 2015, 23:21 Steven Silvester notifications@github.com
wrote:

@tonysyu https://github.com/tonysyu, thanks! (I forgot to mention I
had to comment out the matplotlib import in my ipython startup file). I
agree about the demos.


Reply to this email directly or view it on GitHub
#3968 (comment)
.

Conflicts:
	lib/matplotlib/backends/web_backend/mpl.js
Conflicts:
	lib/matplotlib/backends/backend_webagg_core.py
Conflicts:
	lib/matplotlib/backends/web_backend/mpl.js
Conflicts:
	lib/matplotlib/backends/web_backend/mpl.js
Conflicts:
	lib/matplotlib/backends/web_backend/mpl.js
Conflicts:
	lib/matplotlib/backends/web_backend/mpl.js
Conflicts:
	lib/matplotlib/backends/web_backend/mpl.js
Conflicts:
	lib/matplotlib/backends/backend_webagg_core.py

Conflicts:
	lib/matplotlib/backends/backend_webagg_core.py
	lib/matplotlib/backends/web_backend/mpl.js
Conflicts:
	lib/matplotlib/backends/web_backend/mpl.js
Conflicts:
	lib/matplotlib/backends/web_backend/mpl.js
@blink1073
Copy link
Member Author

I was waiting for #3965 to merge before updating the UAT, now done.

tacaswell added a commit that referenced this pull request Jan 11, 2015
ENH : Add Support for `scroll_event` in WebAgg and NbAgg
@tacaswell tacaswell merged commit b9eb1ff into matplotlib:master Jan 11, 2015
tacaswell added a commit that referenced this pull request Jan 11, 2015
ENH : Add Support for `scroll_event` in WebAgg and NbAgg
@tacaswell
Copy link
Member

back-ported as 22eb1d5

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

5 participants