Skip to content

Conversation

@Light2Dark
Copy link
Contributor

@Light2Dark Light2Dark commented Nov 11, 2024

📝 Summary

Fixes #2060 . Adding MathJax library in PlotlyPlugin to render LaTeX for Plotly.

🔍 Description of Changes

  • Using MathJax 2 library from https://www.mathjax.org/#gettingstarted . Should use v3 in the future once all v2 features are ported over.
  • created a custom hook as the uidotdev useScript hook isn't suitable when there are multiple components calling useScript at the same time

📋 Checklist

  • I have read the contributor guidelines.
  • For large changes, or changes that affect the public API: this change was discussed or approved through an issue, on Discord, or the community discussions (Please provide a link if applicable).
  • I have added tests for the changes made.
  • I have run the code and verified that it works as expected.

📜 Reviewers

WIP

@vercel
Copy link

vercel bot commented Nov 11, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
marimo-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 14, 2024 1:03am
marimo-storybook ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 14, 2024 1:03am

@Light2Dark Light2Dark marked this pull request as draft November 11, 2024 16:59
@Light2Dark Light2Dark changed the title (draft) feat: add mathjax lib using useScript feat: add mathjax lib using useScript Nov 11, 2024
mscolnick
mscolnick previously approved these changes Nov 13, 2024
src: string,
options: ScriptOptions = defaultOptions,
) {
const [status, setStatus] = React.useState<ScriptStatus>();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in order to avoid an extra render, can you actually check the status from the start

useState<ScriptStatus>(() => {
    return document.querySelector(`script[src="${src}"]`)?.dataset.status ?? 'loading'l
})


if (script) {
const domStatus = script.dataset.status;
if (domStatus) {
Copy link
Contributor

@mscolnick mscolnick Nov 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we end early and not observer if its already 'ready'/errored?

Copy link
Contributor

@mscolnick mscolnick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice work, looks great!

@Light2Dark
Copy link
Contributor Author

Light2Dark commented Nov 14, 2024

sorry I missed out the earlier stuff, you're right on the rerenders! Do you think I should add tests? I'm looking into it

@mscolnick
Copy link
Contributor

mscolnick commented Nov 14, 2024

@Light2Dark - up to you if you want to add a test. our hook tests have been pretty minimal, but i have found Claude to be pretty good at writing tests for hooks (smaller than this).
im going to merge, but you are welcome to in a follow-up, thanks!

in short: if its painless and easy, please do, otherwise don't stress over it

@mscolnick mscolnick merged commit 53744aa into marimo-team:main Nov 14, 2024
21 of 23 checks passed
@github-actions
Copy link

🚀 Development release published. You may be able to view the changes at https://marimo.app?v=0.9.19-dev5

@Light2Dark Light2Dark deleted the add-mathjax-plotly branch January 15, 2025 00:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

LaTeX displayed as raw text instead of math formulae in plotly

2 participants