Add support for indentalign and indentshift #118
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ercentage widths to parent elements.
zorkow
approved these changes
Aug 9, 2018
Member
zorkow
left a comment
zorkow
left a comment
There was a problem hiding this comment.
Mostly trivialities.
One suggestion: Introduce a constant on bbox for the '100%' string.
| // Force inheritance of shift and align values (since they are needed to output tables with labels) | ||
| // but make sure they are not given explicitly on the <mtable> tag. | ||
| // | ||
| for (const name of ['indentalign', 'indentalignfirst', 'indentshift', 'indentshiftfirst']) { |
Member
There was a problem hiding this comment.
Would it make sense to put the 'indentXY' elements into a constant list, so it can be reused. It is used again below,
mathjax3-ts/output/chtml/Wrapper.ts
Outdated
| */ | ||
| protected getAlignShift() { | ||
| let {indentalign, indentshift, indentalignfirst, indentshiftfirst} = | ||
| this.node.attributes.getList('indentalign', 'indentshift', |
mathjax3-ts/output/chtml/Wrapper.ts
Outdated
| } | ||
|
|
||
| /* | ||
| * @param{N} chtml The HTML node whose indenting is to be adjusted |
| const chtml = this.chtml; | ||
| const adaptor = this.adaptor; | ||
| const attributes = this.node.attributes; | ||
| const display = (attributes.get('display') === 'block'); |
Member
Author
There was a problem hiding this comment.
I've always preferred them when assigning a variable the result of a boolean operation. It makes the order of operations explicit. I can remove if you think it is important, but we discussed this once before, and you indicated that it was OK and that we could make it part of our style definition if I wanted.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds support for
indentalignandindentshift, which control the alignment and indenting of displayed equations. This is handled in themathwrapper, and also in themtablewrapper since tables with labels are in full-width elements, and the alignment of the actual table must be inside the full-width container. Stylesheet CSS handles the alignment, but the shifting requires explicit styles. Because tables rely on theindentalignandindentshiftvalues, they need to be added to the mtable inheritance.This PR also includes fixes for the bubbling up of full-width element, like tables with labels. So if a table is inside another element (e.g.,
mstyle, orsemantics), that won't disrupt the label positioning. This was being done by hand inmrowelements, but that has now been moved to the wrapper super-class.