Eliminate compatibility issues in supporting notebook and jupyterlab #1902
Conversation
|
|
Formgrader could be loaded in one of two different contexts: on its own from a notebook, or inside an iframe a lab. When navigating to a page outside the formgrader application, this presents an issue in that these two contexts need different navigation mechanisms. Previously, the mechanism was decided by looking at the name of the jupyter-server application. If it was `jupyter-notebook` (notebook<7), use href. Otherwise, use postMessage. However, it's possible to use the notebook interface without `jupyter-notebook`, e.g. `jupyter-server`. Now the mechanism is decided by context clues inside the browser, i.e. whether formgrader is inside an iframe. This commit adds a `linkTo` function which is applied to jQuery elements via `.map` and applies either href or postMessage nav mechanisms based on whether the current window is the top window. Additionally, it completely removes the `is_lab` and `is_jlab` checks as they were relatively naive and add complexity that doesn't need to be there.
| url = self.settings.get("mathjax_url", "") | ||
| if not url or url_is_absolute(url): | ||
| return url | ||
| return url_path_join(self.base_url or "/", url) |
There was a problem hiding this comment.
This method exists in the parent JupyterHandler and could be removed here completely if base_url didn't include .rstrip("/").
There was a problem hiding this comment.
Do you think that rstrip is necessary ? Does it break something if we remove it ?
There was a problem hiding this comment.
The rstrip was added really early in development, so the whole server extension is built based on it. I don't think it will be difficult to change, but every usage of {{ base_url }}/ in the templates would need to be changed to {{ base_url }}, and similar updates need to be made in the javascript too.
|
The two "Check Release" failures and the "Make SDist" failure are related to Automattic/node-canvas. I think this might need a yarn.lock bump, but I'm not sure how safe that is. The test_nbgrader(docs) failure is a timeout because checking GitHub links in the changelog keeps getting rate limited and eventually times out. It seems like it might be worth adding a filter which auto-approves links that start with github.com/search. |
|
@meeseeksdev backport to main |
|
Owee, I'm MrMeeseeks, Look at me. There seem to be a conflict, please backport manually. Here are approximate instructions:
And apply the correct labels and milestones. Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon! Remember to remove the If these instructions are inaccurate, feel free to suggest an improvement. |
Formgrader could be loaded in one of two different contexts: on its own from a notebook, or inside an iframe a lab. When navigating to a page outside the formgrader application, this presents an issue in that these two contexts need different navigation mechanisms.
Previously, the mechanism was decided by looking at the name of the jupyter-server application. If it was
jupyter-notebook(notebook<7), use href. Otherwise, use postMessage. However, it's possible to use the notebook interface withoutjupyter-notebook, e.g.jupyter-server.Now the mechanism is decided by context clues inside the browser, i.e. whether formgrader is inside an iframe.
This commit adds a
linkTofunction which is applied to jQuery elements via.mapand applies either href or postMessage nav mechanisms based on whether the current window is the top window.Additionally, it completely removes the
is_labandis_jlabchecks as they were relatively naive and add complexity that doesn't need to be there.Fixes #1888