feat: adding support to markdown cells (#247)

[MarcosVn]

Written partially the solution for the #247.

some issues still need to be discussed.

[echarles]

Taking back comments from #247 (comment) here

Even implementing the MarkdownCellCode instance, the cell is not rendered as expected (resulting in a blank output). Could it be something related to the execution interface, for example? I noticed that in markdown cells, the interface that transforms the input into a result appears to be quite different;

This cell is probably there, but you need to set the css height in Cell.tsx

        '& .jp-MarkdownCell': {
          height: 'auto !important',
          position: 'relative',
        },

I think the ideal would be to rename the CellAdapter _codeCell property to _cell, however, I was a bit hesitant to make this modification due to not knowing the possible impacts;

Yes, agree for a rename.

I believe we will have other areas to evolve as well, such as in Cell.tsx, where we will need to consider the case of autoStart (perhaps point 1 above is related to this).

I took over your changes and had to case the _cell based on the type to make it build. I guess you will need to do that, storing the _type as a field in CellAdapter driving to manual conditions like e.g.

  execute = () => {
    if (this._type === 'code') {
      CodeCell.execute(this._codeCell as CodeCell, this._sessionContext);
    }
  };

I have a basic Markdown cell which lacks the Markdown rendering

You have to inspire from the code in NotebookAdapter to add the getMarked(), so changing const rendermime = new RenderMimeRegistry({ initialFactories }); to

    this._rendermime = new RenderMimeRegistry({
      initialFactories,
      latexTypesetter: new MathJaxTypesetter(),
      markdownParser: getMarked(languages),
    });

[MarcosVn]

Thank you very much for the feedback @echarles!

I have implemented the suggested changes (except for renaming _codeCell to _cell) and I already have a cell that can submit the markdown and get it rendered.

I am still working on some extra points that are not 100% okay, such as:

• When the markdown is rendered and clicking on the cell, it should go back to the input/editor;
• Submitting the cell with shift + enter (I am also evolving the CellCommands for this point).
• The cell height is not adjusting 100% automatically;

I will continue working on these points, but if you have any more direct suggestions, I would appreciate it!

One last issue that bothered me a bit is that I had to condition many places to the type of the cell (both in the CellAdapter and in Cell). Do you see a major problem with that?

[echarles]

Awesome!

When the markdown is rendered and clicking on the cell, it should go back to the input/editor;

There is probably a listener to be added. The best would be to look at the core jupyterlab notebook and see how it is done there to reuse the existing machinery https://github.com/jupyterlab/jupyterlab/tree/main/packages/notebook-extension/src

Submitting the cell with shift + enter (I am also evolving the CellCommands for this point).

Yes, CellCommands is where you have to add that shortcut.

The cell height is not adjusting 100% automatically;

You mean when you resize the browser? of when you change the content of the cell?

One last issue that bothered me a bit is that I had to condition many places to the type of the cell (both in the CellAdapter and in Cell). Do you see a major problem with that?

No ideal, but good for now.

[MarcosVn]

@echarles, I believe I have addressed all the points we discussed. I am doing another round of review and testing, and I will communicate here shortly so that a review can be done when possible.

[MarcosVn]

@echarles,
I submitted a version that is functional and I believe it is ready for review/testing.

Documenting a few points here:

You mean when you resize the browser? of when you change the content of the cell?

When we change the content of the cell. For some reason this also only happens when the toolbar is active. In my use case, I initially don't want to use the default toolbar, so I "partially" created an interface to allow us to parameterize some UI aspects, such as whether the toolbar will be included in the BoxPanel or not. Please let me know if you disagree.

The best would be to look at the core jupyterlab notebook and see how it is done there to reuse the existing machinery

I looked into the core packages and did not find any registered command that does this. It seems the implementation is indeed through event detection in the DOM and changing the state (rendered) flag. The implementation I did in (Cell.tsx) follows this approach.

In general, I also know that part of the code can be improved/enhanced, but I tried to do it in a "minimally viable" way without injecting too much complexity into the current code and components. Please point out anything you disagree with, and I will gladly make the adjustments.

[echarles]

Thank you so much @MarcosVn. I have checked the source and that looks good to me. We can for sure enhance further after.

I have run the CI and it was failing..

Failed tasks:

• @datalayer/jupyter-react:build
  Error: Process completed with exit code 1.

I will still merge as it and come with a follow-up PR if needed.

[echarles]

LGTM Thx @MarcosVn

[MarcosVn]

Thank you!

[echarles]

@MarcosVn fyi 8e688c9 fixes the build.