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

User defined default viewer take precedence for rendered factory #11541

Merged

Conversation

fcollonval
Copy link
Member

References

Fix #7776

Code changes

Look first in user overrides list for rendered widget so it takes precedence.

User-facing changes

Some link may open with a different editor if they change the default viewer settings.

Backwards-incompatible changes

N/A

@jupyterlab-probot
Copy link

Thanks for making a pull request to jupyterlab!
To try out this branch on binder, follow this link: Binder

@fcollonval fcollonval added this to the 4.0 milestone Nov 25, 2021
@jtpio jtpio changed the title User defined default viewer take precendence for rendered factory User defined default viewer take precedence for rendered factory Nov 29, 2021
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!

@github-actions
Copy link
Contributor

Benchmark report

The execution time (in milliseconds) are grouped by test file, test type and browser.
For each case, the following values are computed: min <- [1st quartile - median - 3rd quartile] -> max.

The mean relative comparison is computed with 95% confidence.

Results table
Test file large_code_notebook large_md_notebook
open
chromium
actual 6320 <- [6937 - 7077 - 7226] -> 7756 3258 <- [3418 - 3491 - 3556] -> 4030
expected 6279 <- [6935 - 7059 - 7157] -> 7751 3298 <- [3429 - 3491 - 3599] -> 3911
Mean relative change 0.3% ± 1.2% -0.6% ± 1.0%
switch-from
chromium
actual 633 <- [725 - 761 - 888] -> 1296 513 <- [562 - 592 - 703] -> 940
expected 654 <- [715 - 779 - 959] -> 1098 516 <- [574 - 666 - 756] -> 856
Mean relative change -2.5% ± 4.1% -4.2% ± 4.1%
switch-to
chromium
actual 800 <- [879 - 914 - 938] -> 1062 661 <- [708 - 736 - 794] -> 859
expected 820 <- [886 - 918 - 950] -> 1075 642 <- [712 - 775 - 812] -> 870
Mean relative change -1.0% ± 1.4% -1.7% ± 2.0%
close
chromium
actual 790 <- [888 - 943 - 1184] -> 1386 605 <- [643 - 659 - 677] -> 761
expected 755 <- [897 - 934 - 1190] -> 1335 607 <- [637 - 655 - 676] -> 766
Mean relative change -0.0% ± 4.5% 0.3% ± 1.3%

Changes are computed with expected as reference.

@fcollonval fcollonval merged commit c65a8e3 into jupyterlab:master Nov 30, 2021
@fcollonval fcollonval deleted the ft/7776-override-rendered-factory branch November 30, 2021 07:34
@krassowski
Copy link
Member

This fixes #10875 - should we backport it?

@fcollonval
Copy link
Member Author

This fixes #10875 - should we backport it?

I don't think it fixes that issue because this addresses a corner case: the viewer to pick when a local file is opened from a link in a markdown cell for example). Before this PR, the user settings was ignored when looking for the viewer is that case.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 30, 2022
@fcollonval
Copy link
Member Author

@meeseeksdev please backport to 3.6.x

@jupyterlab jupyterlab unlocked this conversation Jan 9, 2023
@jupyterlab jupyterlab locked as resolved and limited conversation to collaborators Jan 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot override the default rendered viewer
3 participants