Correct (type-wise) raisebox's usage of sizing's buildHtml. #1361
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1361 +/- ##
==========================================
+ Coverage 81.22% 81.23% +0.01%
==========================================
Files 77 77
Lines 4224 4227 +3
Branches 731 731
==========================================
+ Hits 3431 3434 +3
Misses 673 673
Partials 120 120
Continue to review full report at Codecov.
|
| size: 6, // simulate \normalsize | ||
| }}, options); | ||
| }, group.mode); | ||
| const body = sizing.htmlBuilder(sizedText, options); |
There was a problem hiding this comment.
How about importing sizingGroup and
const newOptions = options.havingSize(6); // simulate \normalsize
const body = sizingGroup([text], newOptions, options);?
There was a problem hiding this comment.
That's true. The reason I did this was so that there's only one place the canonical implementation lives. That way, if it's ever updated in sizing.js we wouldn't have to remember to update it here as well. What do you think?
Currently, functions defined in functions/* import all exports from buildHtml and buildMathML, but they should never use `groupTypes` directly as it loses type-safety. They should instead use more type-safe `htmlBuilder`s and `mathmlBuilder`s exported directly from other definitions `functions/*` to allow flow to catch errors.
| @@ -1,8 +1,8 @@ | |||
| /** | |||
| * This file does the main work of building a domTree structure from a parse | |||
| * tree. The entry point is the `buildHTML` function, which takes a parse tree. | |||
| * Then, the buildExpression, buildGroup, and various groupTypes functions are | |||
| * called, to produce a final HTML tree. | |||
| * Then, the buildExpression, buildGroup, and various groupBuilders functions | |||
There was a problem hiding this comment.
Renaming groupTypes to groupBuilders is an improvement.
There was a problem hiding this comment.
Thoroughly agree! That was @ylemkimon's idea in the last PR.
| size: 6, // simulate \normalsize | ||
| }}, options); | ||
| }, group.mode); |
There was a problem hiding this comment.
Nice use of ParseNode here.
There was a problem hiding this comment.
It's required by the type system. ;)
No description provided.