thebe icon indicating copy to clipboard operation
thebe copied to clipboard

Add functions to handle new code cells in thebe.

Open xr4dsh opened this issue 2 years ago • 6 comments

Add 2 functions that allow adding and removing of code cells in thebe without reconnecting every time. I added a new example to the simple apps to showcase it.

xr4dsh avatar Jan 01 '24 22:01 xr4dsh

Thanks for submitting your first pull request! You are awesome! :hugs:
If you haven't done so already, check out EBP's Code of Conduct and our Contributing Guide, as this will greatly help the review process.
Welcome to the EBP community! :tada:

welcome[bot] avatar Jan 01 '24 22:01 welcome[bot]

Hi @xr4dsh and thank you for the PR!

I've run this locally and the demo works well 👏

Looking at the implementation, I see you're recreating all cells each time through a "shadow" notebook which is then discarded, while that works it requires a lot of setup and as more and more cells are added of course means more of the domis being replaced each time.

Did you consider a more targeted addCell(s), removeCell(s) approach that just adds and renders the cell that is needed?

e.g. you easily add a cell to the existing notebook by just creating a new cell (like in https://github.com/executablebooks/thebe/blob/6e49692a79a8e2d723fee07e8c291bd1b75d0e32/packages/core/src/notebook.ts#L40) and then assigning the session to it, as you are now.

The removeCell could by done by just removing from the notebook.cells array and removing the DOM elements, which feels like that could be done in the removeCell function.

We could also consider extending the ThebeNotebook interface to include an addCell(...) and removeCell(id: string) helper functions in thebe-core if this more targeted approach makes sense for you

stevejpurves avatar Jan 04 '24 12:01 stevejpurves

Thank you for the answer.

The Problem that i am trying to solve is that the web page changes constantly and currently i just rerun all cells when the web page changed, what i really want is that the code cell are re-detected and only the newly added once are run, while the old cell just get their output displayed again, but the ids change when i update the web page, at the moment i just give each code cell an unique html dom id in the backend and when they updated just send back the result with that html id.

Probably thebe is not the best option for me, it would be enough if i could just execute the code in the backend, but I also want working ipywidgets, and this does not seem possible with other approaches.

I have to agree with you that a more dedicated addCell(id) removeCell(id) redetectCell(HtmlDomId) syntax would be much better for this.

xr4dsh avatar Jan 05 '24 11:01 xr4dsh

The Problem that i am trying to solve is that the web page changes constantly... in the sample you included there are add and remove cell buttons, is that the functionality you are trying to implmement or are you doing something beyond that e.g. that some other javascript / process might update the page without a page refresh?

stevejpurves avatar Jan 05 '24 11:01 stevejpurves

Yes, i only import it and the website runs a lot of JavaScript. I am implementing an extension https://github.com/xr4dsh/CodeRunner for https://github.com/oobabooga/text-generation-webui and the biggest problem is that the webpage updates constantly without that my script can notice it, currently i just have a MutationObserver and rerun everything when the site updates.

The example is only for this repo to test if my implementation works at all.

xr4dsh avatar Jan 05 '24 12:01 xr4dsh

Hi @xr4dsh I'm still hesitant to bring these changes into thebe as the way they are implemented are not ideal.

It seemed like the problem you were working around is more to do with the page lifecycle that you are dealing with and it might be more appropraite for you to use thebe-core and thebe-react directly, and manage the page integration yourself at a lower level?

It's been a while since your last comment, maybe you've made progress on this independently?

stevejpurves avatar Mar 15 '24 10:03 stevejpurves

closing as inactive and probably best if the consuming code handles this

stevejpurves avatar Sep 05 '24 15:09 stevejpurves