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

Remove jQuery & jQuery UI #17086

Merged
merged 6 commits into from Jun 8, 2020
Merged

Remove jQuery & jQuery UI #17086

merged 6 commits into from Jun 8, 2020

Conversation

QuLogic
Copy link
Member

@QuLogic QuLogic commented Apr 9, 2020

PR Summary

This is the last step to removing jQuery and jQuery UI. It's not totally finished yet, as the resizing is a bit broken in nbAgg (though okay with plain WebAgg.) I'm also not sure if ResizeObserver is portable enough to rely on by itself (it's about 75% available on caniuse).

Waiting on #17078.

PR Checklist

  • Has Pytest style unit tests
  • Code is Flake 8 compliant
  • [N/A] New features are documented, with examples if plot related
  • [N/A] Documentation is sphinx and numpydoc compliant
  • [N/A] Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • [N/A] Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

@tacaswell
Copy link
Member

Does this need a re-base now that the other PRs are merged?

@QuLogic
Copy link
Member Author

QuLogic commented Apr 29, 2020

Yes, I'll rebase it. I'm also looking into a bug with resize on the notebook version.

@QuLogic
Copy link
Member Author

QuLogic commented Apr 29, 2020

Okay, I think the problem is that in jQuery UI we had start/stop resize events in which we disabled mouse events, but we don't have those any more and get into some kind of loop between our resize and the user resize. I'll need to figure out a way to do that using the newer API.

@QuLogic
Copy link
Member Author

QuLogic commented Apr 30, 2020

I've fixed webagg, but nbagg is broken and I'm not sure what's wrong.

When opening a new figure, in webagg, it looks like:

  1. Python sends a resize message of 640x480 with forward=True.
  2. Both canvas and its surrounding div are resized to 640x480.
  3. The ResizeObserver is triggered and requests Python to resize to 640x480.
  4. Python sends a resize message of 640x480, but with forward=False.
  5. Only the canvas is resized to 640x480, and the div is left alone.

In nbagg though, something else happens before all that, and it works like:

  1. ResizeObserver requests Python to resize to 600x606.5
  2. Python sends a resize message of 640x480 with forward=True.
  3. Both canvas and its surrounding div are resized to 640x480.
  4. The ResizeObserver is triggered and requests Python to resize to 640x486.5.
  5. Python sends a resize message of 600x606 (from message 1), but with forward=False.
  6. Only the canvas is resized to 600x606, and the div is left alone.
  7. The ResizeObserver is triggered and requests Python to resize to 600x612.5.
  8. Python sends a resize message of 640x486 (from message 4), but with forward=False.
  9. Only the canvas is resized to 640x486, and the div is left alone.
  10. The ResizeObserver is triggered again, and loops, alternating from the original initial JavaScript and the initial Python size + 6.5.

The height of the figure just grows and grows and grows after that, unless you grab the resizer with the mouse and shrink it a little. Then the loop stops and everything eventually syncs up together. 600x600 is the initial value given in the JavaScript, though it's strange that webagg ignores it. It's also strange that the height is 6.5 pixels taller than it should be.

Initially, I thought the problem was that the notebook's CSS set all elements to box-sizing:border-box, which would add an extra 2 pixels to the canvas_div due to the 1px border. But setting it back to box-sizing:content-size and/or specifying the box model in the ResizeObserver does not fix anything, and it doesn't really explain the extra 4.5 pixels.

I guess there's something in the notebook JavaScript that's resizing the canvas_div, but I don't know what.

@QuLogic
Copy link
Member Author

QuLogic commented Apr 30, 2020

Also, if I take out the initial sizing on the JavaScript side entirely, then something on the notebook resizes the div to 0x0, and then resizes it again to 300x156.5. I don't even know where those numbers come from.

@tacaswell
Copy link
Member

Pushing this to 3.4.

@tacaswell tacaswell modified the milestones: v3.3.0, v3.4.0 Apr 30, 2020
@QuLogic QuLogic modified the milestones: v3.4.0, v3.3.0 May 20, 2020
@QuLogic QuLogic marked this pull request as ready for review May 20, 2020 04:19
@QuLogic
Copy link
Member Author

QuLogic commented May 20, 2020

Success! For me this now works in WebAgg, the embedding example, and the notebook. Maybe we can get it into 3.3 now.

@tacaswell
Copy link
Member

The save button does not work in nbagg, but works in the embedded webagg example and via plt.show()

@QuLogic
Copy link
Member Author

QuLogic commented May 22, 2020

It doesn't fail for me; is it popup-blocked or something?

@tacaswell
Copy link
Member

It appears to be browser dependent (works in firefox, does not work in brave).

@tacaswell tacaswell added the Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. label May 27, 2020
We only really need clearfix, and some kind of style for the header. We
could drop these entirely, but it's necessary to use these names for
compatibility with the nbAgg styles.
It's now unused, except specfically in the IPython inline figure HTML,
but we know it's available there.
Resizing is really weird if they are aligned at the default
bottom/baseline.
This reverts the notebook's blanket change of all elements to border-box
sizing.
Directly resize the canvas when the container div is resized, instead of
waiting for the message round-trip to Python. Set a minimum size to
prevent positive-finite errors in the console.
@tacaswell
Copy link
Member

power-cycled to pick up the new commits on master (to fix CI)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GUI: nbagg GUI: webagg Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants