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

Support \cfrac #1392

Merged
merged 4 commits into from
Jun 3, 2018
Merged

Support \cfrac #1392

merged 4 commits into from
Jun 3, 2018

Conversation

ronkok
Copy link
Collaborator

@ronkok ronkok commented Jun 1, 2018

Much of this comes directly from #135, so credit to @kevinbarabash.

This was an easier PR to write than #135 since KaTeX now has well established methods for pt-to-em conversions and nulldelimiter.

As in #135, this PR does not support the LaTeX optional argument for numerator justification. It supports a \cfrac #1 #2 syntax.

Fixes #85.

This was an easier PR to write than #135 since KaTeX now has well established methods for pt-to-em conversions and nulldelimiter.

As in #135, this does not support the LaTeX optional argument for numerator justification.  It supports a `\cfrac #1 #2 ` syntax.
@ronkok
Copy link
Collaborator Author

ronkok commented Jun 1, 2018

The screenshot below compares a \frac to a \cfrac.

cfrac

@codecov
Copy link

codecov bot commented Jun 1, 2018

Codecov Report

Merging #1392 into master will decrease coverage by 0.08%.
The diff coverage is 37.5%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1392      +/-   ##
==========================================
- Coverage   82.33%   82.25%   -0.09%     
==========================================
  Files          77       77              
  Lines        4241     4248       +7     
  Branches      731      735       +4     
==========================================
+ Hits         3492     3494       +2     
- Misses        647      650       +3     
- Partials      102      104       +2
Impacted Files Coverage Δ
src/ParseNode.js 88.88% <ø> (ø) ⬆️
src/functions/genfrac.js 60.36% <37.5%> (-2.14%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3d5826c...2a854f5. Read the comment docs.

@kevinbarabash
Copy link
Member

@ronkok here are the updated screenshots:

Chrome
fractiontest-chrome

Firefox
fractiontest-firefox

if (group.value.rightDelim == null) {

if (group.value.continued) {
rightDelim = buildCommon.makeSpan([]); // zero width for \cfrac
Copy link
Member

Choose a reason for hiding this comment

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

I thought this might cause \left/\right delimiters to not render, but it doesn't. I guess the left/right delimeters for genfrac are for a \choose b. Do we need a similar if (group.value.continued) for leftDim up above?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't think so. In LaTeX, they write a \kern-\nulldelimiterspace, but only on the right side of the fraction, not on the left side. We get the same behavior by omitting the space taken by our \nulldelimiter.

Copy link
Member

@kevinbarabash kevinbarabash left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for reviving this code and finishing it off.

@kevinbarabash kevinbarabash merged commit 08b0045 into KaTeX:master Jun 3, 2018
@ronkok ronkok deleted the cfrac branch June 3, 2018 20:46
@prlbr
Copy link

prlbr commented Jun 4, 2018

Thank you!

@edemaine
Copy link
Member

edemaine commented Jun 4, 2018

Happy to see this work, which dates back to 2014, come to fruition! Thanks @ronkok and @kevinbarabash!

@kevinbarabash
Copy link
Member

I can't believe it's been that long.

@OrkhanAlikhanov
Copy link

Thank you both! This was the first reason why I didn't use KaTeX instead of MathJax.

@edemaine
Copy link
Member

@OrkhanAlikhanov Great! Let us know if there are other sticking points you know of, or discover.

@OrkhanAlikhanov
Copy link

@edemaine Hey! I've removed MathJax and integrated KaTeX. Everything seems okay so far. Rendering is now super fast! Especially in android and ios apps. Thank you all!

@edemaine
Copy link
Member

edemaine commented Jul 5, 2018

@OrkhanAlikhanov Glad to hear it!!

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.

None yet

5 participants