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

Fix default viewers override #6923

Merged
merged 3 commits into from Jun 19, 2023
Merged

Fix default viewers override #6923

merged 3 commits into from Jun 19, 2023

Conversation

jtpio
Copy link
Member

@jtpio jtpio commented Jun 15, 2023

Troubleshooting #6914

Notebook 7 should respect the defaultViewers settings overrides when opening a document.

Testing with Jupytext with the following:

  • pip install jupytext
  • pip install git+https://github.com/mwouts/jupyterlab-jupytext.git@jupyterlab4

@jtpio jtpio added the bug label Jun 15, 2023
@jtpio jtpio added this to the 7.0 milestone Jun 15, 2023
@github-actions
Copy link
Contributor

Binder 👈 Launch a Binder on branch jtpio/notebook/default-viewers

@jtpio jtpio changed the title Fix default viewers overrides Fix default viewers override Jun 15, 2023
@jtpio
Copy link
Member Author

jtpio commented Jun 15, 2023

Still troubleshooting this.

@jtpio
Copy link
Member Author

jtpio commented Jun 15, 2023

Looking better:

jupytext-default-viewer.mp4

Although not sure why there is no toolbar when opening the markdown file with the Jupytext Notebook factory. This also seems to be an issue with JupyterLab:

default-viewers-jupytext-jupyterlab.mp4

@jtpio
Copy link
Member Author

jtpio commented Jun 16, 2023

cc @parmentelat for awareness, since this PR should already help in case you would like to try it out.

As noted above, it seems like there is an issue with the toolbar even with JupyterLab. Maybe this could be addressed as part of the JupyterLab 4 compatibility happening in mwouts/jupyterlab-jupytext#3

const urlParams = new URLSearchParams(parsed.search);
let defaultFactory = docRegistry.defaultWidgetFactory(path).name;

// Explicitely get the default viewers from the settings because
Copy link
Member Author

@jtpio jtpio Jun 16, 2023

Choose a reason for hiding this comment

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

Maybe we can track that in a JupyterLab issue so it can be discussed.

@jtpio jtpio marked this pull request as ready for review June 16, 2023 13:55
Copy link
Contributor

@andrii-i andrii-i left a comment

Choose a reason for hiding this comment

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

Main needs to be merged in, otherwise looks good. Thank you @jtpio

@jtpio
Copy link
Member Author

jtpio commented Jun 19, 2023

Thanks @andrii-i for the review!

Just rebased and CI all green ✔️

@jtpio jtpio merged commit d069211 into jupyter:main Jun 19, 2023
24 checks passed
@jtpio jtpio deleted the default-viewers branch June 19, 2023 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants