Skip to content

Conversation

AgentHagu
Copy link
Contributor

What is the purpose of this pull request?

  • Documentation update
  • Bug fix
  • Feature addition or enhancement
  • Code maintenance
  • DevOps
  • Improve developer experience
  • Others, please explain:

Overview of changes:
Fixes #2672

Repeating what was mentioned in the original issue:

When a user does serve or build, the fonts folder that holds the KaTeX fonts is placed inside the css folder, i.e. _site\markbind\css\fonts.

The file markbind.min.css is attempting to access the fonts but is using the wrong relative path to it. That is because within our own core-web package, the original path was packages\core-web\dist\css\markbind.min.css and the path to the fonts folder was packages\core-web\dist\fonts. Because of this, markbind.min.css references the fonts using the path ../font_name

This means that after the site is built, rather than accessing the fonts folder properly, markbind.min.css is trying to access _site\markbind\fonts\font_name, which doesn't exist. This leads to the errors we see in the console. Manually moving the fonts folder out to _site\markbind and reloading the page seems to fix the issue.

The fix simply adjusts the destination path of the fonts to match with our core-web package's directory structure, i.e. fonts folder will now be built outside of the css folder:

Current resulting directory structure Suggested fix's directory structure
image image

This would align with the relative path used by markbind.min.css to access the font files, which means math equations should be properly rendered as expected.

Anything you'd like to highlight/discuss:
It's possible that adjusting the destination path of the fonts folder like this may cause conflicting issues elsewhere in the codebase (though, it's unlikely that the fonts folder is referenced elsewhere).

Testing instructions:

  1. Run npm run build:backend
  2. Create a test site using MarkBind's MathDelimiters plugin, such as:
\begin{equation}
  a^2+b^2=c^2
\end{equation}

\begin{equation}
  \begin{pmatrix}
    A & B \\ B & C
  \end{pmatrix}
\end{equation}
  1. Verify that the math equations are rendered properly using KaTeX's fonts:
Current After Fix
Image Image

Notice the difference in the matrix and Sigma, as well as some minor differences in the other text and notation.

Proposed commit message: (wrap lines at 72 characters)
Fix KaTeX font issue

Adjusts the destination path of the fonts folder to align with
the core-web package's directory structure, allowing proper
access of the fonts for rendering KaTeX.


Checklist: ☑️

  • Updated the documentation for feature additions and enhancements
  • Added tests for bug fixes or features
  • Linked all related issues
  • No unrelated changes

Reviewer checklist:

Indicate the SEMVER impact of the PR:

  • Major (when you make incompatible API changes)
  • Minor (when you add functionality in a backward compatible manner)
  • Patch (when you make backward compatible bug fixes)

At the end of the review, please label the PR with the appropriate label: r.Major, r.Minor, r.Patch.

Breaking change release note preparation (if applicable):

  • To be included in the release note for any feature that is made obsolete/breaking

Give a brief explanation note about:

  • what was the old feature that was made obsolete
  • any replacement feature (if any), and
  • how the author should modify his website to migrate from the old feature to the replacement feature (if possible).

@AgentHagu
Copy link
Contributor Author

The tests are currently failing as there's a difference between the expected site and the commit's site directory structure.

I'd like to verify the following: since this change involves moving all the fonts to a different destination path, should I keep/commit the changes to the font files after running npm run updatetest? I'm a little hesitant since that will mean 1000+ changes

image

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.

KaTeX fonts not loading properly
1 participant