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

Update to JupyterLab v3.1 and support RTC #142

Merged
merged 11 commits into from
Aug 30, 2021
Merged

Update to JupyterLab v3.1 and support RTC #142

merged 11 commits into from
Aug 30, 2021

Conversation

hbcarlos
Copy link
Member

@hbcarlos hbcarlos commented Aug 9, 2021

This is a draft PR to update to JupyterLab v3.1 and add support for RTC.

Due to some attributes on the notebook's metadata (kernelspecs and language_info), we don't share the notebook's metadata by default.

TODO:

  • Upgrade dependencies to 3.1.
  • Use metadata from shared models.
  • Check that the metadata from the shared model is saved to disk.
  • Check that the nbconvert template works.
  • Check that the classic notebook extension works.

Demo:

demo_voila-gridstack_rtc.mp4

@hbcarlos hbcarlos marked this pull request as draft August 9, 2021 13:47
@hbcarlos
Copy link
Member Author

hbcarlos commented Aug 12, 2021

This PR is ready. We need to wait for two PRs in JupyterLab:

Then we can remove:

// TODO: Remove @ts-ignore
// @ts-ignore

and
// TODO: Remove ban-ts-comment
'@typescript-eslint/ban-ts-comment': 'off',

Some of the changes I made:

  • Changed the default behavior of items to locked for a better user experience. The user can still move and resize them, but the other items don't move when moving/resizing one item.
  • Added a shared attribute executed, used to track if when opening Voila-Gridstack editor another client already executed the cells. Otherwise, every time a client connects, executes all cells.
  • Added undo/redo feature.
  • Added a warning dialog to advise the user that the compact layout action will only work with unlocked cells.

@hbcarlos hbcarlos marked this pull request as ready for review August 25, 2021 13:38
@jtpio
Copy link
Member

jtpio commented Aug 27, 2021

Thanks for this!

We need to wait for two PRs in JupyterLab:

Looks like these PRs have now been merged and released. Maybe we can do one more bump to 3.1.9 before merging this one?

@jtpio
Copy link
Member

jtpio commented Aug 27, 2021

We could also add the jupyterlab-link-share extension to the Binder config, so folks can easily try it out on Binder.

@jtpio jtpio added the enhancement New feature or request label Aug 27, 2021
@hbcarlos
Copy link
Member Author

Hey, great to see you are back!

Looks like these PRs have now been merged and released. Maybe we can do one more bump to 3.1.9 before merging this one?

Yes, we need to update to 3.1.9 because the @jupyterlab/testutils v3.1.7 was empty and I had to use the 3.1.6.

I'll do it today.

.eslintrc.js Outdated
Comment on lines 38 to 39
// TODO: Remove ban-ts-comment
'@typescript-eslint/ban-ts-comment': 'off',
Copy link
Member

Choose a reason for hiding this comment

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

Should this be removed now that it has been updated to 3.1.9?

Copy link
Member

Choose a reason for hiding this comment

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

Following-up on the comment: #142 (comment)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, good catch!

Copy link
Member

@jtpio jtpio left a comment

Choose a reason for hiding this comment

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

Thanks, just tested locally and it works great!

@jtpio jtpio merged commit 99c1b35 into master Aug 30, 2021
@jtpio jtpio deleted the update branch August 30, 2021 08:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants