Skip to content

nbagg: Don't close figures for bubbled events. #18639

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 1 commit into from
Oct 5, 2020

Conversation

QuLogic
Copy link
Member

@QuLogic QuLogic commented Oct 2, 2020

PR Summary

In JavaScript, the event handlers are called for both the element and also bubbled up from any children. If the Matplotlib figure is the only thing in the output, that's fine, as closing the figure when one of the canvas or divs clears is the same as when the OutputArea is cleared. However, if there are other outputs, such as widgets, we do not want to close the figure if one of them is cleared.

As there's no way to clear just the figure output, only close the figure if the entire OutputArea div is cleared (i.e., the element we've attached to).

Fixes #18638.

PR Checklist

  • Has pytest style unit tests (and pytest passes).
  • Is Flake 8 compliant (run flake8 on changed files to check).
  • [n/a] New features are documented, with examples if plot related.
  • [n/a] Documentation is sphinx and numpydoc compliant (the docs should build without error).
  • Conforms to Matplotlib style conventions (install flake8-docstrings and pydocstyle<4 and run flake8 --docstring-convention=all).
  • [n/a] New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
  • [n/a] API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).

In JavaScript, the event handlers are called for both the element and
also bubbled up from any children. If the Matplotlib figure is the only
thing in the output, that's fine, as closing the figure when one of the
`canvas` or `div`s clears is the same as when the `OutputArea` is
cleared. However, if there are other outputs, such as widgets, we do not
want to close the figure if one of them is cleared.

As there's no way to clear just the figure output, only close the figure
if the _entire_ `OutputArea` div is cleared (i.e., the element we've
attached to).

Fixes matplotlib#18638.
@QuLogic QuLogic added this to the v3.3.3 milestone Oct 2, 2020
@@ -48,7 +48,7 @@ mpl.mpl_figure_comm = function (comm, msg) {
console.error('Failed to find cell for figure', id, fig);
return;
}
fig.cell_info[0].output_area.element.one(
fig.cell_info[0].output_area.element.on(
Copy link
Member Author

Choose a reason for hiding this comment

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

Note, since we are now ignoring some events, we can't just attach on the first trigger (.one), but all of them (.on)

@dopplershift
Copy link
Contributor

Travis passed, status just didn't update.

@dopplershift dopplershift merged commit 895c4f8 into matplotlib:master Oct 5, 2020
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Oct 5, 2020
@QuLogic QuLogic deleted the nbagg-unbubble branch October 5, 2020 20:19
dopplershift added a commit that referenced this pull request Oct 5, 2020
…639-on-v3.3.x

Backport PR #18639 on branch v3.3.x (nbagg: Don't close figures for bubbled events.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

matplotlib>=3.3.2 breaks ipywidgets.interact
3 participants