Improve MathML for environments #1910
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1910 +/- ##
==========================================
+ Coverage 94.77% 94.82% +0.05%
==========================================
Files 79 79
Lines 4723 4776 +53
Branches 822 842 +20
==========================================
+ Hits 4476 4529 +53
Misses 223 223
Partials 24 24
Continue to review full report at Codecov.
|
src/environments/array.js
Outdated
|
|
||
| // {aligned} and {alignedat} get 0 column spacing. | ||
| // The others get 1em. | ||
| table.setAttribute("columnspacing", group.isAlign ? "0em" : "1em"); |
There was a problem hiding this comment.
Is postgap somehow related to this?
There was a problem hiding this comment.
Yes. The HTML part of the code creates a colsep span whose width is either 0, or 1em (1qquad), or the default arraycolsep. The align environment gets a pregap defined as 1em, but I cannot see how this makes any difference. The end result of all the colsep, arraycolsep, logic works out in effect to just two choices and I chose to simplify it into this one line of code.
I have plans to add more environments. I will revisit this line of code when I do that.
There was a problem hiding this comment.
I figured out what pregap does. The latest commit improves the column spacing for the aligned environment.
src/environments/array.js
Outdated
| @@ -25,6 +25,10 @@ export type AlignSpec = { type: "separator", separator: string } | { | |||
| postgap?: number, | |||
| }; | |||
|
|
|||
| // Constants to indicate column separation | |||
| const ALIGN = 1; | |||
There was a problem hiding this comment.
| const ALIGN = 1; |
src/environments/array.js
Outdated
| @@ -25,6 +25,10 @@ export type AlignSpec = { type: "separator", separator: string } | { | |||
| postgap?: number, | |||
| }; | |||
|
|
|||
| // Constants to indicate column separation | |||
| const ALIGN = 1; | |||
| const ALIGNAT = 2; | |||
There was a problem hiding this comment.
| const ALIGNAT = 2; | |
| export type ColSeparationType = "align" | "alignat"; |
src/environments/array.js
Outdated
| hskipBeforeAndAfter?: boolean, | ||
| addJot?: boolean, | ||
| cols?: AlignSpec[], | ||
| arraystretch?: number, | ||
| colSeparationType?: number, |
There was a problem hiding this comment.
| colSeparationType?: number, | |
| colSeparationType?: ColSeparationType, |
There was a problem hiding this comment.
Is this needed? It seems there is no parseArray calls with colSeparationType.
src/environments/array.js
Outdated
| let columnLines = ""; | ||
| let prevTypeWasAlign = false; | ||
| let iStart = 0; | ||
| let iEnd = group.cols.length; |
There was a problem hiding this comment.
| let iEnd = group.cols.length; | |
| let iEnd = cols.length; |
src/environments/array.js
Outdated
| spacing += i % 2 ? "0em " : "1em "; | ||
| } | ||
| table.setAttribute("columnspacing", spacing.trim()); | ||
| } else if (group.colSeparationType === ALIGNAT) { |
There was a problem hiding this comment.
| } else if (group.colSeparationType === ALIGNAT) { | |
| } else if (group.colSeparationType === "alignat") { |
src/environments/array.js
Outdated
| @@ -448,6 +566,7 @@ const alignedHandler = function(context, args) { | |||
| postgap: 0, | |||
| }; | |||
| } | |||
| res.colSeparationType = isAligned ? ALIGN : ALIGNAT; | |||
There was a problem hiding this comment.
| res.colSeparationType = isAligned ? ALIGN : ALIGNAT; | |
| res.colSeparationType = isAligned ? "align" : "alignat"; |
src/parseNode.js
Outdated
| @@ -28,6 +28,7 @@ type ParseNodeTypes = { | |||
| type: "array", | |||
| mode: Mode, | |||
| loc?: ?SourceLocation, | |||
| colSeparationType?: number, | |||
There was a problem hiding this comment.
| colSeparationType?: number, | |
| colSeparationType?: ColSeparationType, |
| @@ -28,6 +28,7 @@ type ParseNodeTypes = { | |||
| type: "array", | |||
There was a problem hiding this comment.
I think it's better to make ColSeparationType a type and import it here like AlignSpec.
|
The most recent commit picks up the review comments. |
|
@ronkok Thank you for your hard work! 😄 |
|
@ylemkimon Thank you for the valuable reviews! |
This PR improves the MathML of array environments. Specifically, it improves
{array}environments\hlineand\hdashline.To aid review, I have changed the page at webmathcomparison.net so that it runs a version of KaTeX that includes this PR.