Add preview for .ipynb files in pl-file-preview #9840
Conversation
| "marked/marked.min.js", | ||
| "notebookjs/notebook.min.js" |
There was a problem hiding this comment.
We should not be loading these unconditionally if there is no notebook to preview; that's a waste of CPU/bandwidth. I'd recommend using dynamicDependencies as documented here: https://prairielearn.readthedocs.io/en/latest/devElements/. Then you can import() them only when there is actually a .ipynb file to preview.
| // Note about notebookjs: I'm not thrilled with how notebook and marked are styling the | ||
| // Markdown. Unfortunately, I don't have the design skills to make it any better than | ||
| // it is right now. Perhaps somebody else can pick up where I left off and make improvements. | ||
| // Similarly, it might be nice to have syntax highlighting for the code blocks; but, that is | ||
| // also a task for another day. | ||
| // One last idea: Do we want a "hide markdown" button (assuming that it is usually the code | ||
| // blocks that are of interest to most instructors)? |
There was a problem hiding this comment.
Generally speaking comments that way "I" or "should we" shouldn't be committed. You can rephrase the first paragraph to read something like "The notebook styling is less than ideal, etc. etc.". You can add a // TODO comment for syntax highlighting if you think it's important. I don't think a "hide markdown" button is necessary now.
yarn.lock
Outdated
| @@ -11413,6 +11444,40 @@ __metadata: | |||
| languageName: node | |||
| linkType: hard | |||
|
|
|||
| "jsdom@npm:^23.2.0": | |||
There was a problem hiding this comment.
It's unfortunate that this pulls in a version of jsdom that's effectively unused; this leaves us with an extra 4MB of dependencies to ship around. I'm wondering if we could resolve this with Yarn resolutions: https://yarnpkg.com/configuration/manifest#resolutions
I'm happy to try out this change and push it to your branch if that's OK with you.
There was a problem hiding this comment.
I'm good with that.
I'll fix the first two issues Tuesday or Wednesday.
nwalters512
changed the title
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #9840 +/- ##
==========================================
+ Coverage 66.41% 66.43% +0.01%
==========================================
Files 453 453
Lines 70289 70289
Branches 5645 5644 -1
==========================================
+ Hits 46681 46694 +13
+ Misses 23184 23171 -13
Partials 424 424 ☔ View full report in Codecov by Sentry. |
…eLearn into zk_add_workspace_preview2
Added a workspace preview for .ipynb files in the pl-file-preview using notebookjs.
Notebookjs uses DOMpurify to sanitize the rendered markdown.
Future work could include:
Better styling of the markdown
Adding syntax highlighting
Adding a button to hide the Markdown blocks (assuming most instructors primarily care about the code blocks).
https://github.com/jsvine/notebookjs