Wrap document fragment where classes, attributes, or styles are applied#1707
Wrap document fragment where classes, attributes, or styles are applied#1707k4b7 merged 4 commits intoKaTeX:masterfrom ylemkimon:document-fragment
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1707 +/- ##
==========================================
+ Coverage 93.89% 93.95% +0.06%
==========================================
Files 78 78
Lines 4585 4584 -1
Branches 811 809 -2
==========================================
+ Hits 4305 4307 +2
+ Misses 246 245 -1
+ Partials 34 32 -2
Continue to review full report at Codecov.
|
| return value; | ||
| }; | ||
|
|
||
| export const assertType = function<T>(val: mixed, Cls: Class<T>): T { |
There was a problem hiding this comment.
I was sure we were using this in more places, but I guess not.
| let newOptions = options.havingStyle(style.sup()); | ||
| const upperGroup = html.buildGroup(group.body, newOptions, options); | ||
| const upperGroup = buildCommon.wrapFragment( | ||
| html.buildGroup(group.body, newOptions, options), options); |
There was a problem hiding this comment.
Why not wrap all fragments in a span?
There was a problem hiding this comment.
@kevinbarabash The spacing would not be applied properly, (can be fixed by #1706) and the result would be more complicated. But code will become simpler and more type-safe.
| describe("A document fragment", function() { | ||
| it("should have paddings applied inside an extensible arrow", function() { | ||
| const markup = katex.renderToString("\\tiny\\xrightarrow\\textcolor{red}{x}"); | ||
| expect(markup).toContain("x-arrow-pad"); |
There was a problem hiding this comment.
These should maybe be snapshot tests so it's easier to see the structure of the markup.
There was a problem hiding this comment.
@kevinbarabash But the markup result is too complicated, with a lot of unnecessary parts.
There was a problem hiding this comment.
That's definitely a downside of fixtures.
| options: Options, | ||
| ): HtmlDomNode { | ||
| if (group instanceof DocumentFragment) { | ||
| return makeSpan([], [group], options); |
There was a problem hiding this comment.
I guess this is equivalent to
return makeSpan([], group.children, options);? Not sure there are any advantages either way, though.
|
Thanks for the PR. Sorry for the delayed review. |
Fixes issues mentioned in #1618. Also a step toward #1618, separating document fragments from DOM nodes.