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

handling notebook_loaded.Notebook events correctly #885

Open
22 of 24 tasks
jcb91 opened this issue Feb 12, 2017 · 10 comments
Open
22 of 24 tasks

handling notebook_loaded.Notebook events correctly #885

jcb91 opened this issue Feb 12, 2017 · 10 comments

Comments

@jcb91
Copy link
Member

jcb91 commented Feb 12, 2017

As discussed in #879, sometimes (particularly with big notebooks, slow connections, etc), nbextensions get loaded before the notebook has finished loading. In other cases, the notebook may have fully loaded before the nbextension gets loaded, so the nbextension will miss the notebook_loaded.Notebook event. Finally, the event is fired again as part of loading checkpoints, so it's not necessarily a one-time-only event! Nbextension code should be able to deal with any of these three scenarios.

The best way to approach this and still get any initialization which needs to be applied to the whole notebook performed correctly seems to be:

  1. make sure any whole-notebook initialization function can be called multiple times without causing problems like duplicating html elements, etc.
  2. bind the initialization function to the notebook_loaded.Notebook event, so that it gets re-initialized correctly whenever the notebook is (re)loaded
  3. As part of the load_ipython_extension function used to load the nbextension, check if Jupyter.notebook._fully_loaded === true, which indicates we missed the first notebook_loaded.Notebook event. If this is true, call the initialization function(s) mentioned in 1 directly

I've had a quick skim through our nbextensions looking for ones which may need their loading behaviour altered, and come up with the following lists:

The following need updates to handle loading correctly:

These I'm uncertain about:

These seem to run at least some code without load_ipython_extension
being called, which I don't think is supposed to happen, so perhaps need a bit
of reworking:

These haven't even been updated to notebook 4.x, so should either be updated or removed:

These already use the correct approach:

  • autoscroll
  • collapsible_headings
  • init_cell
  • nbTranslate
  • toc2

And these don't need notebook loaded to initialize:

  • addbefore
  • autosavetime
  • chrome-clipboard
  • code_font_size
  • code_prettify
  • comment-uncomment
  • contrib_nbextensions_help_item
  • css_selector
  • datestamper
  • dragdrop
  • equation-numbering
  • gist_it
  • help_panel
  • hide_header
  • hinterland
  • keyboard_shortcut_editor
  • limit_output
  • navigation-hotkeys
  • nbTranslate
  • nbviewer_theme
  • no_exec_dunder
  • notify
  • printview
  • qtconsole
  • rubberband
  • runtools
  • scratchpad
  • search-replace
  • select_keymap
  • skill
  • snippets_menu
  • snippets
  • spellchecker
  • toc2
  • toggle_all_line_numbers
  • tree-filter
  • zenmode
@odedbd
Copy link

odedbd commented Feb 13, 2017

I tested hide_input and I can confirm that it does not handle the refresh properly on a large notebooks.

@jcb91 jcb91 mentioned this issue Feb 13, 2017
@kukanya
Copy link
Contributor

kukanya commented Feb 14, 2017

scroll_down -ok but needs update to handler for new '.output' instances

Could you elaborate a little bit more on that? I don't exactly understand what I need to fix.

@jcb91
Copy link
Member Author

jcb91 commented Feb 14, 2017

So, I may have misunderstood, but I think the code which attaches the resize handler (in scroll_down/main.js#L47-L54 attaches it to any elements with class output that exist when the call is made (i.e. when the nbextension loads), but not to any cells created afterwards (which may, as a result of the conditions discussed above, also include some or all of the cells in the notebook still being loaded).

I'd suggest that the solution is either to catch create_cell.Cell events to attach the handler to new cells as well, or to attach the handler to a parent element like #notebook-container with a selector (see jquery API docs for details of this last).

Does that make sense?

@juhasch
Copy link
Member

juhasch commented Feb 15, 2017

Let's remove the stale history and read-only extensions.
For read-only using the freeze extension is better and has more functionality.

Btw. in the notebook 5.0 there will be is_deletable() and is_editable() functions for each cell and the corresponding metadata metadata.deletable and metadata.editable.
See https://github.com/jupyter/notebook/blob/master/notebook/static/notebook/js/cell.js

@soamaven
Copy link
Contributor

Could this be why my hidden input cells are not hidden on reload (have a large notebook)?

@jcb91
Copy link
Member Author

jcb91 commented May 16, 2017

@soamaven if you're still using an older version, perhaps. However, also possibel is that the large notebook causes the nbextension loading to take too long, and so it gets abandoned by requirejs, as in #822. Do you see any log messages in the
javascript console?

@soamaven
Copy link
Contributor

Using the latest version on conda,
jupyter-contrib-nbextensions==0.2.8

I don't see anything obvious, maybe the script error for 'typo/typo' near the end?

Paste is Here

@jcb91
Copy link
Member Author

jcb91 commented May 17, 2017

Hmm, yeah nothing obvious in the console logs, certainly. I think we've already fixed hide_input to handle the events correctly, but perhaps there's some interaction with another extension... do you still see the same behaviour with all other nbextensions disabled?

@soamaven
Copy link
Contributor

Hey thanks that worked! Most of the extensions play nice, looks like jupyter-js-widgets/extension was the problem

@jcb91
Copy link
Member Author

jcb91 commented May 17, 2017

oh, weird, I've no idea why that should be interacting there 😕

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

No branches or pull requests

5 participants