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

Cell height is off by as much as 30+x #200

Open
igloo1505 opened this issue Feb 14, 2024 · 3 comments
Open

Cell height is off by as much as 30+x #200

igloo1505 opened this issue Feb 14, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@igloo1505
Copy link

igloo1505 commented Feb 14, 2024

Description

When rendering markdown, some (most... maybe all to some extent) cells calculate their height to be significantly larger than what's expected. The values are coming from a hook that runs on scroll as well as after the initial render at line 866 in @jupyterlab/ui-components/lib/components/windowedlist.js where const estimatedTotalHeight = this.viewModel.getEstimatedTotalSize(); returns a value that makes zero sense.

Expected behavior

Display output in a cell that more or less fits the content of that cell.

Context

I'm using this in a fairly complicated application with lots of other css potentially effecting things, but as this is a value that is directly calculated in javascript and appended to the element instead of a css value the only way I can think of for that to effect things is if some css value is causing some weird measurement within that getEstimatedTotalSize function.

  • Datalayer version: ^0.7.14
  • Operating System and version: MacOS Sonoma 14.0
  • Browser and version: Chrome & Safari
@igloo1505 igloo1505 added the bug Something isn't working label Feb 14, 2024
@echarles
Copy link
Member

The values are coming from a hook that runs on scroll as well as after the initial render at line 866 in @jupyterlab/ui-components/lib/components/windowedlist.js where const estimatedTotalHeight = this.viewModel.getEstimatedTotalSize(); returns a value that makes zero sense.

Thx for your deep analysis. Is this something we can help in this repo, or a bug to be fixed in the upstream jupyterlab code base?

@igloo1505
Copy link
Author

That's Jupyerlab code, maintained by Jupyter, but the bug doesn't exist in jupyterlab itself. There must be some way that the value is either being misused, manipulated, or it's not being called at the proper time and it isn't capable of grabbing the data it needs to calculate the proper value. I ended up writing some css that sets the output cells to be closed, and then a sort of hack-y function that sets it to be open after calculating the height, but it's by no means ideal.
I don't want to dissect your library, I mean I've built a pretty large app that relies on it heavily and it works great for the most part, but have you considered extracting the logic for the app into a sort of 'headless' Jupyter? From there different hooks and components could be implemented for React, Angular, Vue, web components and who knows what else, all using the same logic?
I really want to emphasize that I'm not criticizing the library at all. This is by far the best React-Jupyter integration out there, but being able to swap out the editor for Monaco, or even conditionally swap out the editor for a Markdown or HTML specific Editor when the cell type is changed, or pre-parse Markdown input cells before they're passed to Jupyter without worrying that future updates would break things would be an absolute game changer.
I emailed you something about what I'm working on a couple months ago, but to refresh your memory I was a web and sort of python developer for the past 8-9 years, but I quit my job almost 2 years ago to work on writing a paper related to my background in physics. Over that time I became really frustrated with Jupyter Book and even Jupyter Notebook as an actual note taking platform, so I started to build my own. It relies on Jupyter heavily, but also allows MDX with embedded notebooks and cells (embedded individual cells really only hit me as a brilliant idea because of your library), and it adds a bunch of useful search features like a whiteboard, hand-drawn notes that are parsed to SVG's, a plugin architecture to hopefully expand it's usefulness and develop a bit of a community around it, and should be able to be generated from just a single config file and a directory of supported file types (mdx, tabular data types, md, .ipynb, video and image files, etc...). Being able to integrate the Monaco editor (I somehow broke the Jupyter-UI syntax highlighting as well) that is already used elsewhere throughout the app, would be phenomenal, but also being able to inject the mdx components to rendered html before being passed to Jupyter would add a ton of functionality and even go beyond the scope of what Jupyter is currently capable of, at least without some finagling.
I know this isn't a small task, but I'd be happy to help if this was something you were interested in. I'm still going back and forth between the app I'm building and the actual paper that I quit my job and literally wound up homeless for so I'm insanely busy, and I don't always have enough battery to even work late into the night, but eventually I was planning to build pretty much what I described above once the app and the build-from-config script is complete. The app is at that stage where it's pretty much done for a V0.1, but moving everything to a mono-repo and splitting everything into individual packages to support this plugin, slot sorta architecture is proving to be incredibly time consuming.
Either way, this is a great library and I hope you keep up the good work. Jupyter itself is awesome but creating a seamless integration with React would add so many possibilities.
Thanks again!

@fcollonval
Copy link
Member

Hey @igloo1505 thanks for reaching out. Did you turn on notebook windowing by choice? I would advice turning it off (what should be the case by default) to avoid entering the complexity of DOM measuring especially when dealing with an heavily customized application.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants