Skip to content
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 katex font KaTeX_Size4-Regular.woff2 #2478

Merged
merged 6 commits into from
Feb 11, 2020

Conversation

tessus
Copy link
Collaborator

@tessus tessus commented Feb 8, 2020

fixes #2477

Laurent, I think we should add all *.woff2 fonts.

@tessus tessus added bug It's a bug mobile All mobile platforms desktop All desktop platforms PR-tested labels Feb 8, 2020
@tessus tessus requested a review from laurent22 February 8, 2020 20:56
@tessus
Copy link
Collaborator Author

tessus commented Feb 8, 2020

I forgot to actually mention you @laurent22

Not sure, if you turned off all notifications.

@tessus
Copy link
Collaborator Author

tessus commented Feb 10, 2020

@laurent22 I still think we should add all *.woff2 fonts. They don't take a lot of space. Otherwise we'll create one PR after the other for every katex function someone uses.

@laurent22
Copy link
Owner

Yes, maybe, but is there any doc about what these extra fonts are for? We had the same four for a long time and it seems it's good enough, so I get the feeling the other ones aren't really needed for us.

@laurent22
Copy link
Owner

The extra font you've added also needs to be registered in katex.js. We don't load all the assets every time, only those relevant to the active plugins within the note, and so plugins need to tell the app what assets they need.

That makes me think that could actually be automated, but for now we need to do that manually.

@tessus
Copy link
Collaborator Author

tessus commented Feb 11, 2020

Ok, I've registered the font in katex.js.

I've also opened an issue with katex to ask for a list/matrix of what the fonts are used for.
Although I seriously doubt they will reply with an explanation.
They probably tell me that if I wanted to use KaTeX properly, I'd have to load all the fonts because that's why they are there and that it was my fault by not using the entire katex package, but just a part of it.
TBH, I'm sure you would dismiss an error report, if a user had deleted a few files from the Joplin app.

I suspect they are just used for different aspects of KaTeX. e.g. if you use a function x,y,z, font A is used, and so on.

@tessus
Copy link
Collaborator Author

tessus commented Feb 11, 2020

@laurent22 this one should also be ready. btw, I've added comments to make adding other fonts easier in the future. You will only have to uncomment the fonts and done.

@laurent22
Copy link
Owner

I've also opened an issue with katex to ask for a list/matrix of what the fonts are used for.
Although I seriously doubt they will reply with an explanation.
They probably tell me that if I wanted to use KaTeX properly, I'd have to load all the fonts because that's why they are there and that it was my fault by not using the entire katex package, but just a part of it.

Hmm, yes that's a good point. I've also made some tests and I see that while having the fonts there would increase the package size, it shouldn't affect performances. So I'm ok with including all the fonts as you've prepared in the build script. If you could uncomment the code, we can merge this.

@tessus
Copy link
Collaborator Author

tessus commented Feb 11, 2020

@laurent22 done

@laurent22
Copy link
Owner

Perfect, thanks!

@laurent22 laurent22 merged commit 018222a into laurent22:master Feb 11, 2020
@tessus tessus deleted the fix/add-katex-font branch February 16, 2020 01:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug It's a bug desktop All desktop platforms mobile All mobile platforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Katex: 3x3 and above matrices render with a break in the delimiters
2 participants