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

Per notebook template #635

Open
maartenbreddels opened this issue Jun 23, 2020 · 0 comments
Open

Per notebook template #635

maartenbreddels opened this issue Jun 23, 2020 · 0 comments

Comments

@maartenbreddels
Copy link
Member

maartenbreddels commented Jun 23, 2020

Now that #208 is merged, we should work towards having a per-notebook template. There should be two ways to specify this:

I believe both options should be possible by default, but it also should be possible to disable this on an application level (e.g. a company/sysadmin does not want this, and it might be a security risk).

Notebook metadata

As described in #629, it makes sense to put this under the metadata.voila.template JSON key in the notebook file.

Query string

To avoid name collisions, and to be consistent with naming, I propose ?voila-template=foo, and document that all voila-... query names are reserved, and the rest is for use of users (when we merge #414).

In terms of priority, it makes most sense to override them in this order:

  • VoilaConfiguration.template - seen as the default
  • Notebook metadata - overrrides the default, see this as a user set template, that matches the design/intent of the user (could be disabled by the voila server).
  • Http query string - overrides everything, useful for testing, demos etc (could also be disabled by the voila server.

Static resources issue

Now, we have a per server template, and a single /voila/static/(.*) tornado hander, that assumes there is only 1 template. We could change this to /voila/static/(?P<template>.+?)/(.*)such that static file resolving works on a per template basis. This does mean that each template needs to hardcode its template name in the URLs in the template:

Now, we have a per server template, and a single /voila/static/(.*) tornado hander, that assumes there is only 1 template. We should change this to /voila/templates/<template_name>/static/(.*)such that static file resolving works on a per template basis. This does mean that each template needs to hardcode its template name in the URLs in the template:

<link href='{{resources.base_url}}voila/static/vuetify-base/index.css' rel="stylesheet">

This will break the idea of an inheriting template being able to override the static files. E.g., if the vuetify-default (which inherits from vuetify-base), wants to override this css file, it always needs to change this HTML tag.

To fix this, we could expose the template variable via Jinja, so we get:

<link href='{{resources.base_url}}voila/templates/{{resources.template}}/static/index.css' rel="stylesheet">

Or add the static_url function that is used in tornado and notebook, e.g.:

<link href="{{ static_url('index.css') }}" rel="stylesheet">

And similarly make our resources.include_url know about this. e.g.:

<link href="{{ resources.include_link('static/index.css') }}" rel="stylesheet">

and

resources.include_css("static/index.css")

Breakage

This does mean, we would break all templates again. Note that we already broke all templates (for the next release), so this is the right moment to do this.

To support template overriding, templates would need to explicitly support this using the static_url(..) or similar methods.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant