Padding over \sqrt and Paths for frac-line #1143

ronkok · 2018-02-05T01:35:09Z

This replaces two earlier PRs, #1131 Change frac-line from SVG line to SVG path, and #1136 Add 0.08em padding above \sqrt line.

It was easier to create a substitute branch than it was to repair my messed up history.

This replaces two earlier PRs.

kevinbarabash · 2018-02-05T02:24:24Z

src/stretchy.js

-    xrightequilibrium: [["baraboveshortleftharpoon",
-        "rightharpoonaboveshortbar"], 1.75, 716],
-    xleftequilibrium: [["shortbaraboveleftharpoon",
-        "shortrightharpoonabovebar"], 1.75, 716],


I don't think these should've been removed. It breaks the ReactionArrows test.

kevinbarabash · 2018-02-05T02:24:34Z

src/stretchy.js

@@ -47,9 +47,6 @@ const stretchyCodePoint: {[string]: string} = {
    xtwoheadrightarrow: "\u21a0",
    xlongequal: "=",
    xtofrom: "\u21c4",
-    xrightleftarrows: "\u21c4",
-    xrightequilibrium: "\u21cc",  // Not a perfect match.
-    xleftequilibrium: "\u21cb",   // None better available.


These probably shouldn't be removed.

ronkok · 2018-02-05T02:32:27Z

@kevinbarabash Thank you. In my frustration, I got hasty.

kevinbarabash · 2018-02-11T20:28:12Z

I've regenerate the screenshots in ronkok#1. It's incredible how many of our screenshots involve fractions. Many of them probably don't need fractions to test what they're testing.

regenerate screenshots

ronkok · 2018-02-11T20:34:06Z

It's incredible how many of our screenshots involve fractions.

I know that I contributed to that. When I was testing stretchy arrows, fractions were a cheap way to test vertical alignment and vertical kerns used for clearances.

codecov · 2018-02-11T20:35:55Z

Codecov Report

Merging #1143 into master will increase coverage by 0.02%.
The diff coverage is 70.37%.

@@            Coverage Diff             @@
##           master    #1143      +/-   ##
==========================================
+ Coverage   79.36%   79.38%   +0.02%     
==========================================
  Files          59       59              
  Lines        3862     3871       +9     
  Branches      648      648              
==========================================
+ Hits         3065     3073       +8     
- Misses        662      663       +1     
  Partials      135      135

Impacted Files	Coverage Δ
src/functions/genfrac.js	`58.94% <ø> (ø)`	⬆️
src/svgGeometry.js	`100% <100%> (ø)`	⬆️
src/stretchy.js	`82.02% <62.5%> (-0.2%)`	⬇️
src/delimiter.js	`41.76% <70.58%> (+1.44%)`	⬆️

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 bceb7bd...3580760. Read the comment docs.

kevinbarabash · 2018-02-11T20:48:15Z

When I was testing stretchy arrows, fractions were a cheap way to test vertical alignment and vertical kerns used for clearances.

I guess you need some way to test clearance on top. It's either \frac or \sqrt.

kevinbarabash · 2018-02-11T20:50:10Z

src/delimiter.js


    if (delim.type === "small") {
        // Get an SVG that is derived from glyph U+221A in font KaTeX-Main.
-        viewBoxHeight = 1000;  // from font
+        viewBoxHeight = 1080;  // 1000 unit glyph height + 80 unit padding.


I think it be good to extract some of these values into constants, e.g.

const emBoxHeight = 1000; const delimiterPadding = 80;

kevinbarabash · 2018-02-11T20:51:37Z

src/delimiter.js

 -2 1.5-4 2.5s-4.167 1.833-6.5 2.5-5.5 1-9.5 1h-12l-28-84c-16.667-52-96.667
 -294.333-240-727l-212 -643 -85 170c-4-3.333-8.333-7.667-13 -13l-13-13l77-155
- 77-156c66 199.333 139 419.667 219 661 l218 661zM702 0H400000v40H742z`;
+ 77-156c66 199.333 139 419.667 219 661 l218 661zM702 80H400000v40H742z`;


If we have const delimiterPadding = 80; replace the 80 in the this string with ${delimiterPadding}... or at the very least add a component stating what the 80 is so that if people need to change the padding in the future it's easy to find all the places that need updating.

kevinbarabash · 2018-02-11T20:52:24Z

src/delimiter.js

        const newOptions = options.havingBaseStyle(delim.style);
        sizeMultiplier = newOptions.sizeMultiplier / options.sizeMultiplier;
-        spanHeight = 1 * sizeMultiplier;
+        spanHeight = 1.08 * sizeMultiplier;
+        texHeight = 1.00 * sizeMultiplier;


It might be useful to extract a constant for 0.08 as well.

kevinbarabash · 2018-02-11T20:54:55Z

src/stretchy.js

-            "viewBox": "0 0 10 10",
-            "preserveAspectRatio": "none",
+    if (className === "vertical-separator") {
+        path = new domTree.pathNode("vertSeparator");


Cool. Thanks for updating vertical separators to be consistent with the horizontal ones.

kevinbarabash · 2018-02-11T20:57:55Z

src/stretchy.js

+
+        path = new domTree.pathNode("stdHorizRule");
+        svgNode = new domTree.svgNode([path], {
+            "width": "400em",
            "height": 5 * lineThickness + "em",


It's interesting that the line thickness changes for horizontal rules but not for vertical separators. Is that just a LaTeX quirk?

The vertical line constant value is what I found in the code when I changed it from a span border to an SVG path. I did not go back to underlying sources to check if that was correct.

kevinbarabash · 2018-02-11T20:59:26Z

src/katex.less

@@ -475,6 +477,8 @@
    .stretchy {
        width: 100%;
        display: block;
+        position: relative;
+        overflow: hidden;


Is this also b/c of a Chrome rendering issue?

No. Those two new lines are only necessary for IE. In earlier work, I had put those two lines of CSS into class hide-tail and then nested hide-tail inside a stretchy span. I found here that I could save one nesting level by writing those two new lines into stretchy. I could possibly refactor some other code and save a nesting level elsewhere, but I've got other priorities right now.

kevinbarabash · 2018-02-11T21:00:39Z

src/svgGeometry.js

+    // It is in a viewBox that is 5x as tall as the line, so that the
+    // full line will be rendered even if the browser rounds down
+    // the enclosing span size.
+    stdHorizRule: "M0 120 V80 H400000 v40 H0 z M0 120 V80 H400000 v40 H0 z",


I assume all uses of 80 here for padding similar to what's in delimiter.js. Maybe we should have a constant for these so that it's easy to update if necessary.

…into paddingAndFracline

ronkok · 2018-02-13T21:52:53Z

Well, I'm out of date with regard to a submodule. I tried running git submodule update --init --recursive but have gotten no relief. I'm stuck.

kevinbarabash · 2018-02-14T04:01:56Z

@ronkok I'll check out your branch tomorrow and see if I can resolve the conflict.

ronkok · 2018-02-15T22:16:31Z

@kevinbarabash I am not able to view the Travis log, but I believe that I have this PR to the point where it passes all tests except screenshotter.

kevinbarabash · 2018-02-15T22:17:59Z

@ronkok thanks for figuring out the merge. I've been slammed at work. Will try to regenerate the screenshots tonight.

update screenshots

kevinbarabash

LGTM Thanks for adding in the padding constants in delimiter.js.

kevinbarabash · 2018-02-17T18:56:06Z

Having a coverage target for patches is silly. I'm going to try to get rid of that check.

Padding over \sqrt and Paths for frac-line

2b17959

This replaces two earlier PRs.

kevinbarabash reviewed Feb 5, 2018

View reviewed changes

Restore reaction arrows

42c07aa

regenerate screenshots

d619c8e

kevinbarabash mentioned this pull request Feb 11, 2018

unicodeTextInMathMode setting #1117

Merged

Merge pull request #1 from Khan/paddingAndFracline_screenshots

1fb6aed

regenerate screenshots

kevinbarabash reviewed Feb 11, 2018

View reviewed changes

kevinbarabash mentioned this pull request Feb 11, 2018

Support more fractions #1152

Closed

kevinbarabash and others added 6 commits February 11, 2018 17:19

Merge branch 'master' into paddingAndFracline

afa4ca1

Set line padding in a constant

d5d66e6

Merge branch 'paddingAndFracline' of https://github.com/ronkok/KaTeX …

fe5d576

…into paddingAndFracline

Merge branch 'master' into paddingAndFracline

46f4cb0

Update with latest master

c9a4900

Fix lint error

c0e0f95

Merge branch 'master' into paddingAndFracline

19dc870

kevinbarabash and others added 2 commits February 15, 2018 23:24

update screenshots

ac7db73

Merge pull request #2 from Khan/paddingAndFracline

3580760

update screenshots

kevinbarabash approved these changes Feb 17, 2018

View reviewed changes

kevinbarabash merged commit 95ffb4f into KaTeX:master Feb 17, 2018

kevinbarabash mentioned this pull request Feb 18, 2018

Fractional borders sometimes don’t appear. #1168

Closed

ronkok deleted the paddingAndFracline branch February 20, 2018 18:56

ronkok mentioned this pull request Jun 1, 2018

Thick / Double Fraction Bar showing up #1114

Closed

snyk-bot mentioned this pull request Feb 23, 2020

[Snyk] Upgrade katex from 0.9.0 to 0.11.1 saurabharch/Rocket.Chat#1

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Padding over \sqrt and Paths for frac-line #1143

Padding over \sqrt and Paths for frac-line #1143

ronkok commented Feb 5, 2018 •

edited

Loading

kevinbarabash Feb 5, 2018

kevinbarabash Feb 5, 2018

ronkok commented Feb 5, 2018

kevinbarabash commented Feb 11, 2018

ronkok commented Feb 11, 2018

codecov bot commented Feb 11, 2018 •

edited

Loading

kevinbarabash commented Feb 11, 2018 •

edited

Loading

kevinbarabash Feb 11, 2018

ronkok Feb 11, 2018

kevinbarabash Feb 11, 2018

kevinbarabash Feb 11, 2018

kevinbarabash Feb 11, 2018

kevinbarabash Feb 11, 2018

ronkok Feb 11, 2018

kevinbarabash Feb 11, 2018

ronkok Feb 11, 2018

kevinbarabash Feb 11, 2018

ronkok Feb 11, 2018

ronkok commented Feb 13, 2018

kevinbarabash commented Feb 14, 2018

ronkok commented Feb 15, 2018

kevinbarabash commented Feb 15, 2018

kevinbarabash left a comment

kevinbarabash commented Feb 17, 2018

Padding over \sqrt and Paths for frac-line #1143

Padding over \sqrt and Paths for frac-line #1143

Conversation

ronkok commented Feb 5, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ronkok commented Feb 5, 2018

kevinbarabash commented Feb 11, 2018

ronkok commented Feb 11, 2018

codecov bot commented Feb 11, 2018 • edited Loading

Codecov Report

kevinbarabash commented Feb 11, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ronkok commented Feb 13, 2018

kevinbarabash commented Feb 14, 2018

ronkok commented Feb 15, 2018

kevinbarabash commented Feb 15, 2018

kevinbarabash left a comment

Choose a reason for hiding this comment

kevinbarabash commented Feb 17, 2018

ronkok commented Feb 5, 2018 •

edited

Loading

codecov bot commented Feb 11, 2018 •

edited

Loading

kevinbarabash commented Feb 11, 2018 •

edited

Loading