-
-
Notifications
You must be signed in to change notification settings - Fork 833
LaTeX rendering in element-web using KaTeX #5244
Conversation
This reverts commit e4448ae.
Might be pertinent to add new config stuff to |
I can't seem to include |
Shifted katex.min.css into webpack.config.js in element-web (see linked PR), which makes most of the tests happy. Don't know why the first 2 are failing and/or if it's my doing. |
@akissinger You can bisect where the problem is by rolling back your branch to an earlier version. If I remember correctly all tests passed when you first submitted this branch. Looking at the failing tests, a node module called dom_handler seems to be broken. No idea how this relates to your work. How does it look, when you build locally? |
Looks like d2054ea is good and build errors where introduced later. |
Tidied the commits and works now. Actual code is identical to before, so I figure it was a one-off problem with the CI server. |
Excellent, I hope your work gets reviewed and merged soon. |
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.
Thanks for your work on this! It's a great start.
The test failure (chains-end-to-end-tests) is probably just because you're based on an older version of |
@@ -76,6 +76,8 @@ | |||
"highlight.js": "^10.1.2", | |||
"html-entities": "^1.3.1", | |||
"is-ip": "^2.0.0", | |||
"katex": "^0.12.0", | |||
"cheerio": "^1.0.0-rc.3", |
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.
Note to the next reviewer: cheerio is already pulled in by another dependency, so this isn't adding a new dependency, but is added here because it is now being used by HtmlUtils
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.
Aside from a translation nit, looks good to me. Seems to do what it says on the box.
This should be re-reviewed by another developer who's more familiar with the react-sdk. It will probably need design/product review before the labs flag is removed.
Thank you for working on this, and sorry for the delay in reviewing it.
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.
The code seems reasonable at first glance, though I'm going to leave it to someone who has a bit more experience with the composer.
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.
Overall, this looks great, and I am excited to try it out! 😄
I believe there are only a few small nits to resolve, and then we should be ready to merge this.
@jryans okay, I think that's all your changes done. I don't know why the new Vercel thing isn't working, but it might just need to be re-merged to develop. in any case, the other tests still pass. |
Ah yeah, sorry about this, I was trialling Vercel to add previews of PR, but it went a bit haywire. I have disabled it now, and the error is fine to ignore. |
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.
Looks great, thanks again for working on this! 😄
nice! it wasn't exactly the one-liner I was expecting, but glad to see it merged. :) |
I'm not familiar with this "Labs" configuration since I'm only using the matrix.org server. I'm guessing it's where the server is configured. As for the matrix.org server, do they plan to enable LaTeX rendering ? |
It is not related to the Matrix server you are using but instead the client you are using. https://develop.element.io has a config.json with Labs enabled. |
I'm using element on Android. There is indeed a Labs settings but so far without an option to enable LaTeX. I guess it should appear there in a later version (I'm using version 1.0.11[401500112], up to date in the beta program from the Android PlayStore). However, I can't find a Labs tab/menu in the desktop client for Linux (from https://element.io/get-started). However, the |
This repository is not related to the Mobile apps. The mobile apps are native, this is a React SDK. For enabling Labs on the Desktop app see https://github.com/vector-im/element-web/blob/develop/docs/config.md and https://github.com/vector-im/element-desktop#user-specified-configjson |
Thanks a lot, LaTeX is now working for me in the desktop client. I submitted a feature request in the Android sdk repository (matrix-org/matrix-android-sdk#566) |
For what it's worth, I implemented support for this in Element Android here: element-hq/element-android#2133 It could do with some polish but it basically works. |
Hm, I couldn't find any config.json in the packaged version of my system, but you can do:
|
This is an implementation of MSC2191: matrix-org/matrix-spec-proposals#2191
It is based on a trimmed-down and updated version of PR #3251, which won't auto-merge any more. It is different in that implements the latest edits (as of Sept 2020) of MSC2191 and does not include GUI configuration (only a global on/off switch in Labs and flags in for custom delimiters in
config.json
).LaTex rendering is implemented using KaTeX 0.12.0, and is turned off by default. It can be switched on by the user in Labs (documentation updated in matched PR element-hq/element-web#15277).
The other relevant setting is
latex_maths_delims
, which allows customization of user-facing LaTeX delimiters. By default, inline-math is wrapped in double-$'s and display in triple-$'s. The inline case matches the conventions of other chat utilities with TeX support, such as Zulip.You can reproduce the default behaviour in
config.json
as follows:The
*_pattern
settings are given explicitly, which are double-quoted strings that get passed toRegExp(..., "mg")
internally. Note backslashes in the regex should be escaped, and literal backslashes in the regex need to be escaped twice.The first match group is HTML-entities encoded and given as the string in
<span data-mx-maths="....">
or<div data-mx-maths="....">
, for inline and display math, respectively.The translation to/from delimited LaTeX to HTML tags described in MSC2191 is done in
src/editor/(de)serialize.ts
. The KaTeX rendering is insrc/HtmlUtils.tsx
.Finally,
src/Markdown.js
is told to ignore these tags, so clients can mix math and markdown in a single message.Signed-off-by: Aleks Kissinger <aleks0@gmail.com>