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

Eliminate compatibility issues in supporting notebook and jupyterlab #1902

Merged
merged 3 commits into from
Aug 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions nbgrader/docs/source/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,10 @@
# If true, do not generate a @detailmenu in the "Top" node's menu.
#texinfo_no_detailmenu = False

linkcheck_ignore = [
# Assume GitHub contributor search links for this repo are always valid
r"https://github.com/search\?q=repo%3Ajupyter%2Fnbgrader\+involves%3A.*type=Issues",
]

# Example configuration for intersphinx: refer to the Python standard library.
intersphinx_mapping = {'http://docs.python.org/': None}
Expand Down
6 changes: 5 additions & 1 deletion nbgrader/server_extensions/formgrader/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

from tornado import web
from jupyter_server.base.handlers import JupyterHandler
from jupyter_server.utils import url_path_join, url_is_absolute
from ...api import Gradebook
from ...apps.api import NbGraderAPI

Expand Down Expand Up @@ -41,7 +42,10 @@ def gradebook(self):

@property
def mathjax_url(self):
return self.settings['mathjax_url']
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)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This method exists in the parent JupyterHandler and could be removed here completely if base_url didn't include .rstrip("/").

https://github.com/jupyter-server/jupyter_server/blob/5fa7ceb3f1a5b103b9eea1231480ebddd9ae76fb/jupyter_server/base/handlers.py#L248-L261

Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think that rstrip is necessary ? Does it break something if we remove it ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.


@property
def exporter(self):
Expand Down
3 changes: 0 additions & 3 deletions nbgrader/server_extensions/formgrader/formgrader.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,6 @@ def load_jupyter_server_extension(nbapp):
nbapp.log.info("Loading the formgrader nbgrader serverextension")
webapp = nbapp.web_app

# Save which kind of application is running : Jupyterlab like or classic Notebook
webapp.settings['is_jlab'] = not (nbapp.name == 'jupyter-notebook')

formgrader = FormgradeExtension(parent=nbapp)

# compatibility between notebook.notebookapp.NotebookApp and jupyter_server.serverapp.ServerApp
Expand Down
4 changes: 1 addition & 3 deletions nbgrader/server_extensions/formgrader/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ def get(self):
"manage_assignments.tpl",
url_prefix=self.url_prefix,
base_url=self.base_url,
is_lab=int(self.settings['is_jlab']),
windows=(sys.prefix == 'win32'),
course_id=self.api.course_id,
exchange=self.api.exchange_root,
Expand Down Expand Up @@ -108,10 +107,9 @@ def get(self, submission_id):
'index': ix,
'total': len(indices),
'base_url': self.base_url,
'my_mathjax_url': self.mathjax_url if self.settings['is_jlab'] else self.base_url + '/' + self.mathjax_url,
'my_mathjax_url': self.mathjax_url,
'student': student_id,
'last_name': submission.student.last_name,
'last_name': submission.student.last_name,
'first_name': submission.student.first_name,
'notebook_path': self.url_prefix + '/' + relative_path
}
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ var AssignmentUI = Backbone.View.extend({
this.listenTo(this.model, "request", this.animateSaving);
this.listenTo(this.model, "sync", this.closeModal);

this.is_lab = options.is_lab || false;
this.render();
},

Expand Down Expand Up @@ -97,22 +96,14 @@ var AssignmentUI = Backbone.View.extend({

render: function () {
this.clear();
var this_assignment = this;

// assignment name
var name = this.model.get("name")
this.$name.attr("data-order", name);

/* Append link with :
* - href if this is a Notebook<7 environment
* - click listener to send message to iframe parent if this is Jupyter Lab environment
*/
this.$name.append($("<a/>")
.text(name)
.attr("target", self.is_lab ? undefined : "_blank")
.attr("href", self.is_lab ? undefined : base_url + "/tree/" + url_prefix + "/" + this.model.get("source_path"))
.click(self.is_lab ? function(){
window.parent.postMessage(jlab_go_to_path(url_prefix + "/" + this_assignment.model.get("source_path")), '*');
} : undefined)
.map(linkTo("directory", url_prefix + "/" + this.model.get("source_path")))
);

// duedate
Expand Down Expand Up @@ -154,16 +145,8 @@ var AssignmentUI = Backbone.View.extend({
// preview student version
var release_path = this.model.get("release_path");
if (release_path) {
/* Append link with :
* - href if this is a Notebook<7 environment
* - click listener to send message to iframe parent if this is Jupyter Lab environment
*/
this.$preview.append($("<a/>")
.attr("target", self.is_lab ? undefined : "_blank")
.attr("href", self.is_lab ? undefined : base_url + "/tree/" + url_prefix + "/" + release_path)
.click(self.is_lab ? function(){
window.parent.postMessage(jlab_go_to_path(url_prefix + "/" + release_path), '*');
} : undefined)
.map(linkTo("directory", url_prefix + "/" + release_path))
.append($("<span/>")
.addClass("glyphicon glyphicon-search")
.attr("aria-hidden", "true")));
Expand Down Expand Up @@ -215,20 +198,20 @@ var AssignmentUI = Backbone.View.extend({
// generate feedback
if (num_submissions > 0) {
this.$generate_feedback.append($("<a/>")
.attr("href", "#")
.attr("href", "#")
.click(_.bind(this.generate_feedback, this))
.append($("<span/>")
.addClass("glyphicon glyphicon-comment")
.append($("<span/>")
.addClass("glyphicon glyphicon-comment")
.attr("aria-hidden", "true")));
}

// feedback
if (num_submissions > 0) {
this.$release_feedback.append($("<a/>")
.attr("href", "#")
.attr("href", "#")
.click(_.bind(this.release_feedback, this))
.append($("<span/>")
.addClass("glyphicon glyphicon-envelope")
.append($("<span/>")
.addClass("glyphicon glyphicon-envelope")
.attr("aria-hidden", "true")));
}

Expand Down Expand Up @@ -579,7 +562,7 @@ var createAssignmentModal = function () {
modal = createModal("add-assignment-modal", "Add New Assignment", body, footer);
};

var loadAssignments = function (is_lab=false) {
var loadAssignments = function () {
var tbl = $("#main-table");

models = new Assignments();
Expand All @@ -592,7 +575,6 @@ var loadAssignments = function (is_lab=false) {
var view = new AssignmentUI({
"model": model,
"el": insertRow(tbl),
"is_lab": is_lab
});
views.push(view);
});
Expand All @@ -605,8 +587,6 @@ var loadAssignments = function (is_lab=false) {
var models = undefined;
var views = [];

var is_lab = is_lab || false;

$(window).on('load', function () {
loadAssignments(is_lab);
loadAssignments();
});
41 changes: 41 additions & 0 deletions nbgrader/server_extensions/formgrader/static/js/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,3 +87,44 @@ var insertDataTable = function (tbl) {
}]
});
};

var linkTo = function (type, path) {
/*
* Connect a link in the appropriate manner for the context.
* - If we're in the outermost frame, assume notebook and use an href.
* - If we're in an iframe, assume lab and send a message.
*/
if (window === window.top) {
var prefix = {
notebook: "/notebooks/",
file: "/edit/",
directory: "/tree/"
}[type];

return (_, el) => {
return $(el)
.attr("href", prefix + path)
.attr("target", "_blank")[0];
};
} else {
var command = {
notebook: "docmanager:open",
file: "docmanager:open",
directory: "filebrowser:go-to-path",
}[type];

return (_, el) => {
return $(el)
.attr("href", "#")
.click(() => {
window.parent.postMessage(
JSON.stringify({
"command": command,
"arguments": {"path": path}
}),
"*"
);
})[0];
}
}
}
1 change: 0 additions & 1 deletion nbgrader/server_extensions/formgrader/templates/base.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
<script src="{{ base_url }}/formgrader/static/components/datatables.net-bs/js/dataTables.bootstrap.min.js"></script>
<script src="{{ base_url }}/formgrader/static/js/backbone_xsrf.js"></script>
<script src="{{ base_url }}/formgrader/static/js/utils.js"></script>
<script src="{{ base_url }}/formgrader/static/js/jupyterlab_utils.js"></script>

<link rel="stylesheet" href="{{ base_url }}/formgrader/static/components/bootstrap/css/bootstrap.min.css" />
<link rel="stylesheet" href="{{ base_url }}/formgrader/static/components/datatables.net-bs/css/dataTables.bootstrap.min.css">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ var base_url = "{{ resources.base_url }}/formgrader";
<script src="{{ resources.base_url }}/formgrader/static/js/formgrade_keyboardmanager.js"></script>
<script src="{{ resources.base_url }}/formgrader/static/js/formgrade_models.js"></script>
<script src="{{ resources.base_url }}/formgrader/static/js/formgrade.js"></script>
<script src="{{ resources.base_url }}/formgrader/static/js/utils.js"></script>
<script type="text/javascript">
function toggle_name(on) {
$(".name-shown").tooltip('hide');
Expand All @@ -33,12 +34,11 @@ function toggle_name(on) {
}
}
function jlab_open_notebook(notebook_path) {
return JSON.stringify({
"command": "docmanager:open",
"arguments": {"path": notebook_path}
$(document).ready(function() {
$("[data-link]").each(function() {
$(this).map(linkTo("notebook", $(this).data("link"))).removeAttr("data-link");
});
}
});
</script>

<link rel="stylesheet" href="{{ resources.base_url }}/formgrader/static/components/bootstrap/css/bootstrap.min.css" />
Expand All @@ -64,20 +64,12 @@ function jlab_open_notebook(notebook_path) {
<li><a href="{{ resources.base_url }}/formgrader/gradebook/{{ resources.assignment_id }}/{{ resources.notebook_id }}">{{ resources.notebook_id }}</a></li>
<li class="active live-notebook">
<a class="name-hidden" data-toggle="tooltip" data-placement="bottom" title="Open live notebook"
{%- if is_lab -%}
onclick='window.parent.postMessage(jlab_open_notebook("{{ resources.notebook_path }}"), "*");'
{%- else -%}
target="_blank" href="{{ resources.base_url }}/notebooks/{{ resources.notebook_path }}"
{%- endif -%}
data-link="{{ resources.notebook_path }}"
>
Submission #{{ resources.index + 1 }}
</a>
<a class="name-shown" data-toggle="tooltip" data-placement="bottom" title="Open live notebook"
{%- if is_lab -%}
onclick='window.parent.postMessage(jlab_open_notebook("{{ resources.notebook_path }}"), "*"); '
{%- else -%}
target="_blank" href="{{ resources.base_url }}/notebooks/{{ resources.notebook_path }}"
{%- endif -%}
data-link="{{ resources.notebook_path }}"
>
{%- if resources.last_name and resources.first_name -%}
{{ resources.last_name }}, {{ resources.first_name }}
Expand Down Expand Up @@ -107,4 +99,4 @@ function jlab_open_notebook(notebook_path) {
$('span.glyphicon.name-hidden').tooltip({title: "Show student name", placement: "bottom", trigger: "hover"});
$('span.glyphicon.name-shown').tooltip({title: "Hide student name", placement: "bottom", trigger: "hover"});
</script>
{% endmacro %}
{% endmacro %}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
{%- block head -%}
<script>
var url_prefix = "{{ url_prefix }}";
var is_lab = {% if is_lab %}true{% else %}false{% endif %};
</script>

<script src="{{ base_url }}/formgrader/static/js/manage_assignments.js"></script>
Expand Down
6 changes: 3 additions & 3 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2133,9 +2133,9 @@ caniuse-lite@^1.0.30001349:
integrity sha512-Sd6pjJHF27LzCB7pT7qs+kuX2ndurzCzkpJl6Qct7LPSZ9jn0bkOA8mdgMgmqnQAWLVOOGjLpc+66V57eLtb1g==

canvas@^2.6.1:
version "2.11.0"
resolved "https://registry.yarnpkg.com/canvas/-/canvas-2.11.0.tgz#7f0c3e9ae94cf469269b5d3a7963a7f3a9936434"
integrity sha512-bdTjFexjKJEwtIo0oRx8eD4G2yWoUOXP9lj279jmQ2zMnTQhT8C3512OKz3s+ZOaQlLbE7TuVvRDYDB3Llyy5g==
version "2.11.2"
resolved "https://registry.yarnpkg.com/canvas/-/canvas-2.11.2.tgz#553d87b1e0228c7ac0fc72887c3adbac4abbd860"
integrity sha512-ItanGBMrmRV7Py2Z+Xhs7cT+FNt5K0vPL4p9EZ/UX/Mu7hFbkxSjKF2KVtPwX7UYWp7dRKnrTvReflgrItJbdw==
dependencies:
"@mapbox/node-pre-gyp" "^1.0.0"
nan "^2.17.0"
Expand Down
Loading