Switched react-katex package #2619
Conversation
akrolsmir
left a comment
akrolsmir
left a comment
There was a problem hiding this comment.
Hi @RedFrez , thanks for your contribution! Really appreciated your explanations of why we're switching to this package and what we're gaining.
It does look like the st_markdown.spec.js needs a minor tweak (I'm amazed at that old misspelling, actually).
|
I was just tracking this down. it seems like whatever generated the MathML version spelled it wrong, and the test just matched it.
LaTeX and TeX also had the same misspellings.
All tests should pass now. The Block math didn't have this issues since it just had math. |
|
@akrolsmir the current test that is failing is with st.file_uploader timed out. Is this a known flaky test? I am unable to rerun the test as the option is disabled for me. |
|
I've triggered a rerun of the test but it should be fixed in the latest |
* Fix slider UX * Add snapshot
* Revert "Deploy button front (streamlit#2552)" This reverts commit 0f66b6e. * Fix merge (erm... revert) conflict. * Revert "Deploy button py (streamlit#2535)" This reverts commit 0bbc7aa. Co-authored-by: karrie <[email protected]>
…treamlit#2604) * Add %u to list of valid number_input formatters * Have number_input warn user of mismatched type and format string * Use more modern f-string syntax instead of .format()
* Add and use sudoless machine id getter for metrics Step 2 of the metrics user ID improvement plan. * Add machine-id v3 tests Duplicates of the v1 tests, since those covered the cases that remain in v3, and we will remove v1 and its tests later. Also fix formatting of some python files. * DRY out machine ID paths into global constants
* Increase side padding to 5rem when app is in wide mode * Upload Cypress snapshot
\streamlit#2395 ended up being caused by this since we don't rerender placeholder components created by `st.empty()` when react props/state change as a performance optimization, but we need to do so when replacing an existing component with an empty one to get rid of what's currently drawn on screen. Closes streamlit#2395
akrolsmir
left a comment
akrolsmir
left a comment
There was a problem hiding this comment.
Hm, it looks like the latest merge has pulled in some unwanted files and the diff has ballooned as a result. Maybe you want to reset the git branch to before the merge, then pull in the latest develop? Something like:
git checkout latex
git reset --hard d3513118887a28a6765fe3998814a35bd08d009d
git merge upstream/develop
git push
|
@akrolsmir I flipped a coin between merge and rebase and I guess the wind blew it the wrong way. Is there somewhere other than the main contributing page that has instructions about creating a pull request? |
Hm, I don't believe so, but we definitely should have one -- thanks for the suggestion! Anyways, this PR looks great; that one test failure is another flake (it's been fixed in the latest develop, I think) so I'm just rerunning for now :P |
kmcgrady
left a comment
kmcgrady
left a comment
There was a problem hiding this comment.
Looks Great! Going to merge this in!
* develop: Display a warning when not run via `streamlit run` (streamlit#2643) Extract a shared radio component for use within our UI (streamlit#2657) 🔧 "Run on save" now defaults to true (streamlit#2641) Bump year to 2021 in license headers (streamlit#2654) Switched react-katex package (streamlit#2619) Don't add license headers for files in node_modules and cypress folders (streamlit#2655)
7 participants
This resolves some issues with the rendering of latex. #2086 and #2556 are both resolved with this fix.
The existing react-katex package seems to have been abandoned 3 years ago. This switches to a newer react-katex package that was inspired by the first. The new package is what is currently recommended on the react-markdown page. This shows the difference between the two packages.
I manually tested the functionality of each issue:
#2086
#2556
Both pytest and jstest ran without problems.
While running the e2e test I did receive this failure:
I ran the e2e on the develop branch to compare and it seems to fail at different places. I am not sure if this is something with my setup or an ongoing issue. I am fairly confident that the issue is not within the changes proposed by this pull request.
Please let me know if there is a better way to test these changes and I can provide the update.