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

⚛️ Render HTML directly #118

Merged
merged 7 commits into from
Sep 14, 2023
Merged

⚛️ Render HTML directly #118

merged 7 commits into from
Sep 14, 2023

Conversation

rowanc1
Copy link
Collaborator

@rowanc1 rowanc1 commented Mar 21, 2023

This is an initial sketch of fixing #64.

  • sanitize html securely using the same Jupyter machinery

In JupyterLab, we should be doing HTML rendering directly, but it does need to be sanitized using Jupyter's machinery. @agoose77 maybe you can help here?

@fwkoch it looks like certain inline elements get split up into different html tags, at least when they are inline. Maybe we just need a transform to get these into one html element, then we can render that easily.

image

@github-actions
Copy link
Contributor

Binder 👈 Launch a Binder on branch executablebooks/jupyterlab-myst/fix/html

@rowanc1 rowanc1 added the enhancement New feature or request label Mar 21, 2023
@agoose77
Copy link
Collaborator

We can get the JupyterLab sanitiser from the renderer options: https://jupyterlab.readthedocs.io/en/stable/api/interfaces/rendermime.irendermime.irendereroptions.html

It looks like the current renderer doesn't expose a mechanism for passing additional arguments. I think that we probably want the sanitisation to happen in the src/renderers.tsx file rather than earlier in the AST building (for obvious reasons), so is there a way to extend this?

@rowanc1
Copy link
Collaborator Author

rowanc1 commented Mar 21, 2023

I am also noticing that .jp-RenderedHTMLCommon is required on the markdown cell for the alert to work. I did not add that as it changes links and quite a few other things that we don't want. This is going to take some other style work. 🤔

image

@rowanc1
Copy link
Collaborator Author

rowanc1 commented Mar 21, 2023

We can also do this much easier (at AST parsing/transformation time) in that place I think we have access to all of the various things we need. We will also have to merge the HTML tokens and sanitize them together.

@nthiery
Copy link

nthiery commented Jun 2, 2023

Hello!

I am starting to plan for our JupyterHub and course upgrades for next Semester
(early September). No pressure intended, but do you have some rough timeline
for this PR?

Thanks in advance,

@nthiery
Copy link

nthiery commented Aug 27, 2023

Hi @rowanc1,

I am preparing the upgrade of our jupyterhub for the upcoming teaching semester
starting September 11th and am facing a dilemma:

  • Using jupyterlab-myst==0.1.7a6 currently forces a downgrade of jupyterlab-server which causes further trouble and incompatibilities down the line
  • Using the latest jupyterlab-myst currently breaks the notebooks of several of our courses that use some inline HTML.
  • Not installing jupyterlab-myst (:fearful:)

Depending on the tentative timeline for this PR, do you have a recommandation for which route to take?

Thanks in advance,

@agoose77
Copy link
Collaborator

@nthiery we can definitely get a release of the old jupyterlab-myst that fixes the jupyter_server limit (although you should be safe to forcibly ignore this), but I'd hope we can move you to the latest. Are there any limitations on the version of JupyterLab you can support?

Meanwhile, I'll let Rowan speak to the HTML side of things.

@nthiery
Copy link

nthiery commented Aug 28, 2023

Thanks @agoose77 for the quick feedback! We will be using jupyterlab 4.*.

@agoose77
Copy link
Collaborator

Can you confirm whether you use inline execution, or is it just MyST rendering?

@nthiery
Copy link

nthiery commented Aug 29, 2023

@agoose77 We just use MyST rendering, no inline expressions (I assume that's what you meant with inline executions).

@nthiery
Copy link

nthiery commented Aug 30, 2023

Just as a data point: I made a minimal install of JupyterLab 4 and jupyterlab-myst 0.1.7a6;
markdown cells are then rendered as empty.

Maintaining the 0.1 line is probably a waste at this stage; hoping for another solution
to be in reach ...

environment:

name: jupyterhub-paris-saclay-test

channels:
- conda-forge

dependencies:
- jupyterlab>=4
- jupytext

- pip
- pip:
  - jupyterlab-myst==0.1.7a6       # Workaround latest mystjs-based implementation not preserving HTML admonitions
  - .
Package list ``` mamba list # packages in environment at /opt/mambaforge/envs/jupyterhub-paris-saclay-test: # # Name Version Build Channel _libgcc_mutex 0.1 conda_forge conda-forge _openmp_mutex 4.5 2_gnu conda-forge anyio 3.7.1 pyhd8ed1ab_0 conda-forge argon2-cffi 23.1.0 pyhd8ed1ab_0 conda-forge argon2-cffi-bindings 21.2.0 py311hd4cff14_3 conda-forge arrow 1.2.3 pyhd8ed1ab_0 conda-forge asttokens 2.2.1 pyhd8ed1ab_0 conda-forge async-lru 2.0.4 pyhd8ed1ab_0 conda-forge attrs 23.1.0 pyh71513ae_1 conda-forge babel 2.12.1 pyhd8ed1ab_1 conda-forge backcall 0.2.0 pyh9f0ad1d_0 conda-forge backports 1.0 pyhd8ed1ab_3 conda-forge backports.functools_lru_cache 1.6.5 pyhd8ed1ab_0 conda-forge beautifulsoup4 4.12.2 pyha770c72_0 conda-forge bleach 6.0.0 pyhd8ed1ab_0 conda-forge brotli-python 1.0.9 py311ha362b79_9 conda-forge bzip2 1.0.8 h7f98852_4 conda-forge ca-certificates 2023.7.22 hbcca054_0 conda-forge cached-property 1.5.2 hd8ed1ab_1 conda-forge cached_property 1.5.2 pyha770c72_1 conda-forge certifi 2023.7.22 pyhd8ed1ab_0 conda-forge cffi 1.15.1 py311h409f033_3 conda-forge charset-normalizer 3.2.0 pyhd8ed1ab_0 conda-forge comm 0.1.4 pyhd8ed1ab_0 conda-forge debugpy 1.6.8 py311hb755f60_0 conda-forge decorator 5.1.1 pyhd8ed1ab_0 conda-forge defusedxml 0.7.1 pyhd8ed1ab_0 conda-forge entrypoints 0.4 pyhd8ed1ab_0 conda-forge exceptiongroup 1.1.3 pyhd8ed1ab_0 conda-forge executing 1.2.0 pyhd8ed1ab_0 conda-forge fqdn 1.5.1 pyhd8ed1ab_0 conda-forge idna 3.4 pyhd8ed1ab_0 conda-forge importlib-metadata 6.8.0 pyha770c72_0 conda-forge importlib_metadata 6.8.0 hd8ed1ab_0 conda-forge importlib_resources 6.0.1 pyhd8ed1ab_0 conda-forge ipykernel 6.25.1 pyh71e2992_0 conda-forge ipython 8.14.0 pyh41d4057_0 conda-forge isoduration 20.11.0 pyhd8ed1ab_0 conda-forge jedi 0.19.0 pyhd8ed1ab_0 conda-forge jinja2 3.1.2 pyhd8ed1ab_1 conda-forge json5 0.9.14 pyhd8ed1ab_0 conda-forge jsonpointer 2.0 py_0 conda-forge jsonschema 4.19.0 pyhd8ed1ab_1 conda-forge jsonschema-specifications 2023.7.1 pyhd8ed1ab_0 conda-forge jsonschema-with-format-nongpl 4.19.0 pyhd8ed1ab_1 conda-forge jupyter-lsp 2.2.0 pyhd8ed1ab_0 conda-forge jupyter-server 1.24.0 pypi_0 pypi jupyter_client 8.3.1 pyhd8ed1ab_0 conda-forge jupyter_core 5.3.1 py311h38be061_0 conda-forge jupyter_events 0.7.0 pyhd8ed1ab_2 conda-forge jupyter_server_terminals 0.4.4 pyhd8ed1ab_1 conda-forge jupyterhub-paris-saclay 0.1 pypi_0 pypi jupyterlab 4.0.5 pyhd8ed1ab_0 conda-forge jupyterlab-markup 2.1.0a1 pypi_0 pypi jupyterlab-myst 0.1.7a6 pypi_0 pypi jupyterlab_pygments 0.2.2 pyhd8ed1ab_0 conda-forge jupyterlab_server 2.24.0 pyhd8ed1ab_0 conda-forge ld_impl_linux-64 2.40 h41732ed_0 conda-forge libexpat 2.5.0 hcb278e6_1 conda-forge libffi 3.4.2 h7f98852_5 conda-forge libgcc-ng 13.1.0 he5830b7_0 conda-forge libgomp 13.1.0 he5830b7_0 conda-forge libnsl 2.0.0 h7f98852_0 conda-forge libsodium 1.0.18 h36c2ea0_1 conda-forge libsqlite 3.43.0 h2797004_0 conda-forge libstdcxx-ng 13.1.0 hfd8a6a1_0 conda-forge libuuid 2.38.1 h0b41bf4_0 conda-forge libzlib 1.2.13 hd590300_5 conda-forge markupsafe 2.1.3 py311h459d7ec_0 conda-forge matplotlib-inline 0.1.6 pyhd8ed1ab_0 conda-forge mistune 3.0.1 pyhd8ed1ab_0 conda-forge nbclient 0.8.0 pyhd8ed1ab_0 conda-forge nbconvert-core 7.8.0 pyhd8ed1ab_0 conda-forge nbformat 5.9.2 pyhd8ed1ab_0 conda-forge ncurses 6.4 hcb278e6_0 conda-forge nest-asyncio 1.5.6 pyhd8ed1ab_0 conda-forge notebook-shim 0.2.3 pyhd8ed1ab_0 conda-forge openssl 3.1.2 hd590300_0 conda-forge overrides 7.4.0 pyhd8ed1ab_0 conda-forge packaging 23.1 pyhd8ed1ab_0 conda-forge pandocfilters 1.5.0 pyhd8ed1ab_0 conda-forge parso 0.8.3 pyhd8ed1ab_0 conda-forge pexpect 4.8.0 pyh1a96a4e_2 conda-forge pickleshare 0.7.5 py_1003 conda-forge pip 23.2.1 pyhd8ed1ab_0 conda-forge pkgutil-resolve-name 1.3.10 pyhd8ed1ab_0 conda-forge platformdirs 3.10.0 pyhd8ed1ab_0 conda-forge prometheus_client 0.17.1 pyhd8ed1ab_0 conda-forge prompt-toolkit 3.0.39 pyha770c72_0 conda-forge prompt_toolkit 3.0.39 hd8ed1ab_0 conda-forge psutil 5.9.5 py311h2582759_0 conda-forge ptyprocess 0.7.0 pyhd3deb0d_0 conda-forge pure_eval 0.2.2 pyhd8ed1ab_0 conda-forge pycparser 2.21 pyhd8ed1ab_0 conda-forge pygments 2.16.1 pyhd8ed1ab_0 conda-forge pysocks 1.7.1 pyha2e5f31_6 conda-forge python 3.11.5 hab00c5b_0_cpython conda-forge python-dateutil 2.8.2 pyhd8ed1ab_0 conda-forge python-fastjsonschema 2.18.0 pyhd8ed1ab_0 conda-forge python-json-logger 2.0.7 pyhd8ed1ab_0 conda-forge python_abi 3.11 3_cp311 conda-forge pytz 2023.3 pyhd8ed1ab_0 conda-forge pyyaml 6.0.1 py311h459d7ec_0 conda-forge pyzmq 25.1.1 py311h75c88c4_0 conda-forge readline 8.2 h8228510_1 conda-forge referencing 0.30.2 pyhd8ed1ab_0 conda-forge requests 2.31.0 pyhd8ed1ab_0 conda-forge rfc3339-validator 0.1.4 pyhd8ed1ab_0 conda-forge rfc3986-validator 0.1.1 pyh9f0ad1d_0 conda-forge rpds-py 0.10.0 py311h46250e7_0 conda-forge send2trash 1.8.2 pyh41d4057_0 conda-forge setuptools 68.1.2 pyhd8ed1ab_0 conda-forge six 1.16.0 pyh6c4a22f_0 conda-forge sniffio 1.3.0 pyhd8ed1ab_0 conda-forge soupsieve 2.3.2.post1 pyhd8ed1ab_0 conda-forge stack_data 0.6.2 pyhd8ed1ab_0 conda-forge terminado 0.17.1 pyh41d4057_0 conda-forge tinycss2 1.2.1 pyhd8ed1ab_0 conda-forge tk 8.6.12 h27826a3_0 conda-forge tomli 2.0.1 pyhd8ed1ab_0 conda-forge tornado 6.3.3 py311h459d7ec_0 conda-forge traitlets 5.9.0 pyhd8ed1ab_0 conda-forge typing-extensions 4.7.1 hd8ed1ab_0 conda-forge typing_extensions 4.7.1 pyha770c72_0 conda-forge typing_utils 0.1.0 pyhd8ed1ab_0 conda-forge tzdata 2023c h71feb2d_0 conda-forge uri-template 1.3.0 pyhd8ed1ab_0 conda-forge urllib3 2.0.4 pyhd8ed1ab_0 conda-forge wcwidth 0.2.6 pyhd8ed1ab_0 conda-forge webcolors 1.13 pyhd8ed1ab_0 conda-forge webencodings 0.5.1 py_1 conda-forge websocket-client 1.6.2 pyhd8ed1ab_0 conda-forge wheel 0.41.2 pyhd8ed1ab_0 conda-forge xz 5.2.6 h166bdaf_0 conda-forge yaml 0.2.5 h7f98852_2 conda-forge zeromq 4.3.4 h9c3ff4c_1 conda-forge zipp 3.16.2 pyhd8ed1ab_0 conda-forge ```

@fwkoch
Copy link
Contributor

fwkoch commented Sep 2, 2023

I've updated this PR with a couple things. Rowan already removed htmlPlugin, which previously converted all html to mdast, and just lost any unsupported html element type, e.g. button. Now, the mdast is also passed through a new transform that resolves broken-up, inline html (referred to in Rowan's original comment on this PR, see jupyter-book/mystmd#575). We also need to add the jupyter html sanitization (I added a separate library to sanitize, which probably shouldn't be landed; it pulls in jsdom... too big!)

@agoose77 - I'll spend a few more minutes to see if I can sort out the sanitizer from jupyter. Then once the myst PR is landed, I'll kick it over to you to look over! Let me know if you think there are any aspects of this I'm totally overlooking...

@nthiery
Copy link

nthiery commented Sep 2, 2023

This sounds very promising! Let me know if beta-testing can help at any point. I am your man :-)

@agoose77
Copy link
Collaborator

agoose77 commented Sep 5, 2023

@fwkoch thanks a bunch for this! I installed a local build of mystmd, and I can see that the inline styles work. The default styling for buttons isn't brilliant - could you take a look at that? I don't know what we're aiming for, but what's there right now doesn't look button-like at all.

I've updated the PR to the changes in main, which brings in the latest MyST-md work.

Another point - could we move myst-theme to 1.x versioning? The existing caret constraint in jupyterlab-myst is too restrictive because myst-theme is on 0.x

@fwkoch
Copy link
Contributor

fwkoch commented Sep 6, 2023

@agoose77 - thanks for jumping into the sanitizer stuff! I played around with simply importing the default sanitizer from @jupyterlab/apputils (instead of dompurify) - but it looks like your solution to inject the sanitizer is much better (and still defaults to that sanitizer probably...?).

Anyway - the new version of myst-transforms (and other myst libs) has been released, so you should be able to update away from your local install.

Regarding myst-theme version I don't really see a problem, @stevejpurves do you have any concerns there?

And as for button styling, I don't have any opinions or experience... Does the default styling match how things used to look in the previous markdown renderer...? If so, maybe it's just good enough, and we can do better-looking MyST buttons in another pass? But maybe it's more than just buttons, e.g. Rowan's comment above about .jp-RenderedHTMLCommon; I have no feeling about what to do there. Maybe you have some opinions on this too @stevejpurves ? 🙂

@agoose77
Copy link
Collaborator

agoose77 commented Sep 6, 2023

Tailwind seems to strip away the button styling, but I don't see any CSS in our stylesheets to restore anything. I assume the expectation is that we add the appropriate tailwind classes, but we'd want to do this ourselves in the MyST stylesheet I think.

Also, I didn't notice that I committed my local paths. Whoops!

@stevejpurves
Copy link
Contributor

We can move myst-theme to 1.0 🥳 whenever we like. @agoose77 sorry I'm catching up, but I didn't really understand the issue you were having with the ^0.x caret

@agoose77
Copy link
Collaborator

agoose77 commented Sep 6, 2023

There's nothing inherently wrong with ^0.x, but it does differ in semantics to ^N.x (N != 0). It wasn't actually this PR that I noticed the problem though, rather this commit where I had to manually bump the constraint because we expect 0 to mean major-version 0.

@stevejpurves
Copy link
Contributor

Also on tailwind, yet is does strip away all base styles. I note some styling of button here https://github.com/executablebooks/jupyterlab-myst/blob/main/style/preflight.css but am not clear on the meaning of preflight?

In any case we can introduce anywhere and apply some basic styles, maybe something like:

.myst button {
   @apply bg-blue-500 hover:bg-blue-700 text-white font-bold py-2 px-4 rounded
}

But adjusting those colors to fit

@agoose77
Copy link
Collaborator

agoose77 commented Sep 6, 2023

My understanding of preflight is that it's a tailwind concept to introduce a clean slate upon which to customise the styling.

@agoose77
Copy link
Collaborator

agoose77 commented Sep 9, 2023

@fwkoch @stevejpurves I thought I'd already written this message but couldn't find it — it's proving tricky to correct the button styles because it transpires that the authors card renders each author as a button. Is there a good reason for this vs an <a> link?

@agoose77
Copy link
Collaborator

@nthiery I'm really conscious that your course is starting soon (tomorrow?). Could you perhaps expand upon whether you'll have <button> elements that we need to fix the styling for?

@nthiery
Copy link

nthiery commented Sep 11, 2023

Hi @agoose77,

I really appreciate all the efforts each of you are putting into this!
My course started today, but I found ways to wiggle around: not updating
our main server to not break other's teachers work; having my own course
notes a bit degraded. In short, I can hold my breath a bit more.

If in the current state it's working for most elements, maybe you
could cut a beta release, and I am happy to deploy it and have
it tested at "large scale" (in terms of users and content) to provide
you feedback.

@agoose77 agoose77 marked this pull request as ready for review September 14, 2023 07:38
@agoose77
Copy link
Collaborator

This isn't "done", but I think an alpha release would be helpful. I'll merge.

@agoose77 agoose77 merged commit c0a92b1 into main Sep 14, 2023
6 checks passed
@agoose77 agoose77 deleted the fix/html branch September 14, 2023 07:38
@nthiery
Copy link

nthiery commented Sep 14, 2023

Yeah! Will try this tonight :-)

@agoose77
Copy link
Collaborator

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

Successfully merging this pull request may close these issues.

5 participants