-
Notifications
You must be signed in to change notification settings - Fork 91
Add scroll_outputs configuration #683
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
Add scroll_outputs configuration #683
Conversation
|
I found more issues in the irsa-tutorials build using this branch and this config turned on: https://output.circle-artifacts.com/output/job/6313d183-aae2-490b-a692-68e1d986d56e/artifacts/0/_build/html/tutorials/euclid_access/1_Euclid_intro_MER_images.html It's not ready for merging until I fix them. |
Simplify repeated long selectors with :is() selector
|
Ok so I fired docs CI at NASA-NAVO/navo-workshop#207 and I'm happy with results now: https://output.circle-artifacts.com/output/job/a8645ce2-d3c0-46f0-8ede-c6e12e9ad527/artifacts/0/_build/html/content/use_case_notebooks/candidate_list_solution.html (this notebook has a good mix of long tables, and images (short and long)) |
| div.cell:is( | ||
| .tag_output_scroll, | ||
| .tag_scroll-output, | ||
| .config_scroll_outputs | ||
| ) | ||
| div.cell_output, | ||
| div.cell.tag_scroll-input div.cell_input { | ||
| max-height: 24em; | ||
| overflow-y: auto; | ||
| max-width: 100%; | ||
| overflow-x: auto; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh! I forgot to mention: this is a little bit tangential to this PR. I realized the style definitions were redundant so I combined input and output styles. And CSS selector was too long so I simplified it with :is()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All I could do is to trust you and the test suite and downstream test PRs on this 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nevermind, it's all in there automatically! Ahh, there is no nb_scroll_outputs documentation anywhere. That should be added somewhere 🤷♀️
Besides that it all looks good to me, the downstream testing was very convincing!
| div.cell:is( | ||
| .tag_output_scroll, | ||
| .tag_scroll-output, | ||
| .config_scroll_outputs | ||
| ) | ||
| div.cell_output, | ||
| div.cell.tag_scroll-input div.cell_input { | ||
| max-height: 24em; | ||
| overflow-y: auto; | ||
| max-width: 100%; | ||
| overflow-x: auto; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All I could do is to trust you and the test suite and downstream test PRs on this 😅
|
Ohh, I found this rename PR from |
|
Thanks @jaladh-singhal! |
Fixes #677
Introduced a new configuration option
scroll_outputsinmyst_nb/core/config.pyto enable scrollable long outputs - configurable at global, file, cell, and render levels.Updated the
myst_nb/core/render.pylogic to apply thetag_output_scrollclass to cells when thescroll_outputsconfiguration is enabled.Added tests for this config: test case in
test_render_outputs.py, input notebookscroll_outputs.ipynband output ASTtest_scroll_outputs.xml. This was heavily inspired from hide_cell_content option PR.[Bonus] Added tags
scroll-output(oroutput_scroll) andscroll-inputin documentation (they was no documentation when they were added)