jupyter-book / jupyterlab-myst

Use MyST Markdown directly in Jupyter Lab
https://jupyter-book.github.io/jupyterlab-myst/
BSD 3-Clause "New" or "Revised" License
140 stars 16 forks source link

Certain inline html not rendering correctly #64

Open fperez opened 1 year ago

fperez commented 1 year ago

Describe the bug

context

I have notebooks with inline html that isn't rendering correctly.

Reproduce the bug

In a markdown cell, the following content:

Use the <button class='btn btn-default btn-xs'><i class='icon-stop fa fa-stop'></i></button> button in the toolbar above.

renders in "normal" JupyterLab correctly as:

image

while mystjs renders it incorrectly as:

image

List your environment

(stat159-s23) longs[~]> jupyter-book --version
Jupyter Book      : 0.13.1
External ToC      : 0.2.4
MyST-Parser       : 0.15.2
MyST-NB           : 0.13.2
Sphinx Book Theme : 0.3.3
Jupyter-Cache     : 0.4.3
NbClient          : 0.5.13
fperez commented 1 year ago

This is with mystjs 0.1.3 btw.

agoose77 commented 1 year ago

@rowanc1 based upon the fact that the https://spec.myst.tools renderer shows the (seemingly) correct AST but non-rendered output, I think this is a mystjs bug rather than something specific to this repo?

rowanc1 commented 1 year ago

Yep, this is going to be a deeper issue in mystjs parser. It was originally all aimed towards secure html / semantic meaning, so we disabled html (both a security hole & difficult to translate to all the various other renderers). @fwkoch is doing surgery in the parser at the moment, and this would be a good thing to consider as well if we can!

Related: executablebooks/jupyterlab-mystjs#34

rowanc1 commented 1 year ago

We fixed up certain pieces in v0.1.4, but it is likely that the <button>/<i> listed in this issue are not going to work.

Currently HTML is actually being parsed and trying to be "understood", rather than fully rendered as is. We are doing this because MyST likes to understand more for rendering out to other formats like Word/LaTeX/JATS. I think that the behaviour in JupyterLab need to mimic a bit closer what happens today, and that is going to be a different chunk of work!

Leaving this open for now, but it should have some improvements in v0.1.4!

mcauthorn commented 1 year ago

One more quick update on this, which I suspect may be useful for those looking for a work around (granted, it's not ideal): If I coerce the rendering by converting it to a code cell and use the IPython magic command:

%%html
<div>
<img src="/files/Jupyter-Logo-DemOS.png" width="250"/>
</div>

The HTML renders as expected.

rowanc1 commented 1 year ago

Another work around in that case is to use MyST, an image or figure directive! :) https://myst-tools.org/docs/mystjs/figures#image-directive

```{figure} https://source.unsplash.com/random/400x200?beach,ocean
:name: myFigure
:alt: Random image of the beach or ocean!
:align: center

Relaxing at the beach 🏝 🌊 😎
nthiery commented 1 year ago

On the same line, plain HTML/bootstrap admonitions such as:

<div class="alert alert-info">

Lorem ipsum ...

</div>

are apparently not supported. Looking at the produced html with firefox's developer tools, the div opening and closing tags are completely removed.

Since HTML admonitions are used quite regularly --it will take time for all material out there to be converted to myst -- I am about to disable jupyterlab-myst on our hub. :cry: Looking forward a transition period where both HTML and myst admonitions are supported! Thanks in advance!

If useful, I can make and provide a screenshot and/or create a separate issue.

Reported by Henri-Jean Garchon

agoose77 commented 1 year ago

@nthiery if you need this feature now, you can temporarily pin jupyterlab-myst==0.1.7a6 which will use the old MarkdownIt implementation. That implementation is less pretty than the 1.0 series, but it should support HTML admonitions IIRC.

Meanwhile, @rowanc1 and the team can give a better idea of if/when we'll be able to bring the 1.0 series to feature parity on this topic. :)

fperez commented 1 year ago

I'll ad to @nthiery's comments above that I hope these use cases can be brought along, even as much as I appreciate that from the developer's perspective, they may be somewhat painful to support. The reality is that a lot of content exists out there that relies on these conventions, and we'll hit significant adoption barriers if the transition path is too bumpy for users who are busy with other things.

It's the tough tradeoff of meeting messy real-world usage! Thanks again team for being so responsive.

rowanc1 commented 1 year ago

Thanks @nthiery and @fperez -- this is absolutely something we will support, and now bumping up to high on the priority list. We will try to get this addressed soon so that @nthiery can reinstall! :)

@nthiery this issue is fine to track the issue, once we start working on it we might ping you in the PR to get a few more of the use-cases to ensure you are covered.

Thanks again both for the feedback!

nthiery commented 1 year ago

@nthiery if you need this feature now, you can temporarily pin jupyterlab-myst==0.1.7a6

Worked smoothly! Thank you @agoose77 !

Looking forward to use the latest jupyterlab-myst ; but no rush: I totally can do with the current solution to enjoy both worlds :-)

agoose77 commented 1 year ago

Good to know, @nthiery! If you're willing to be a tester, it would be great to get your feedback once we have some progress here; I would like to stop maintaining jupyterlab-myst<1 ASAP, so the sooner we're at feature parity the better! :)

nthiery commented 1 year ago

I am your man. Tell me when I should test again!

firasm commented 1 year ago

I was just about to create an issue saying that my <img src... tags weren't working after installing jupyterlab-myst, but installing jupyterlab-myst==0.1.7a6 instead made it work. Thanks for the workaround and I see that there's a draft PR already!

P.S. For context, I'm trying to just jupyterlab-deck and see how it works in conjunction with JupyterBook and Jupyterlab-myst.

nthiery commented 1 year ago

Just for the record: HTML links and <kbd> tags are not rendered as well.

nthiery commented 1 year ago

Also I had to stop using jupyterlab-myst==0.1.7a6 for one of my courses because it forced a downgrade of jupyter-server from 2.5 to 1.23.6 which caused other issues.

So now you have a dedicated early adopter of the latest 1.1.* line :-)

nthiery commented 11 months ago

I have installed the latest alpha release with #118 merged in, and played with it on my course notes on my machine. It looks very promising. So far I noticed only the following glitches:

I have deployed it in our computer lab for testing by 50 students during our next session, 30 minutes from now. Stay tuned :-)

agoose77 commented 11 months ago

Ah yes, I can see why the img tags don't load; we need to use our link transformer. I'll take a peek at how we can do this.

rowanc1 commented 11 months ago

👋 @agoose77 I think that for both of these, especially the math (or any myst/md inside html), it might be worth taking the approach of converting to named-html nodes (with various properties, that have mdast children) rather than a single HTML blob that is rendered out. This would mean no more special case for the image nodes, which is already handled. This does require additional work in the renderer, but I think is pretty close to the transform that @fwkoch put together.

Looking forward to more feedback @nthiery from your class!

nthiery commented 11 months ago

Sorry, no additional feedback. It just worked :-)

rowanc1 commented 11 months ago

Have played around with @fwkoch's approach for HTML rendering this afternoon, and I don't think that I can improve it that much. (i.e. I don't think the approach I described above is a good plan anymore.) I did fix up some style issues around kbd, button etc. in #186, these should now be the same as native Jupyter.

There are a lot of edge cases that I found around block html elements (e.g div) that render differently in jupyter vs github vs myst. For example, the <div>$Ax=b$</div> example @nthiery renders as plain HTML with dollars, and then happens to be post-processed by mathjax, which is not the case for example with <div>_underscores shown_</div> which shows the underscores. If you put a new-line between the div and the content, then this will render the markdown in both.

One future challenge that I am not sure how/if we want to solve is having the myst-style content rendered inside of an HTML block, e.g. a hover cross-reference or abbreviation. Or do we just do the most basic HTML rendering when you have opted to use HTML directly. If we go with myst-style content inside a div, this syntax becomes much closer to mdx.

nthiery commented 11 months ago

Hello! One piece of feedback after all, from testing all week long:

<kbd>z</kbd> renders as: image instead of just: image

rowanc1 commented 11 months ago

I cannot seem to to reproduce this @nthiery:

image

Which version are you working with?

nthiery commented 11 months ago

Ah, yes, sorry, I missed some context. The failing kbd's actually were sitting within a div. So this must be an avatar of the previously analyzed examples. And I can easily fix that on my side by using a proper admonition.

<div>

<kbd>z</kbd>;

</div>
psychemedia commented 3 months ago

I still see an issue trying to load images in using HTML <img> tag, though the markdown renderer works.

The main reason against me using the markdown format is the image styling - I really need to be able to set the width, which the markdown syntax doesn't support.