Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix several issues with tables (label alignment in CHTML, lines in SVG, centering). (mathjax/Mathjax#2601) #668

Merged
merged 5 commits into from
Apr 14, 2021

Conversation

dpvc
Copy link
Member

@dpvc dpvc commented Mar 29, 2021

This PR fixes a number of issues with table output:

  • Labels in CHTML could not align with their rows (Multiline labels on the {align} environment are rendered with offset MathJax#2601). This was due to accumulated round-off problems, which is resolved by explicitly setting the row heights regardless of whether they should already be correct or not.
  • When rows have rowalign specified, the height and depth weren't properly computed (it was always computed as though alignment were baseline).
  • Cells with rowalign set to top, bottom, or center could cause the height and depth of the row to be too large (the sizes were always computed as through the alignment were baseline).
  • The center alignment was incorrectly called middle, so didn't work properly in SVG.
  • The positioning of a row label in CHTML wasn't always correct when rowalign was specified.
  • In SVG, row lines on labeled rows where not visible (the CSS wasn't being properly applied to them).

@dpvc dpvc added this to the 3.1.3 milestone Mar 29, 2021
@dpvc dpvc requested a review from zorkow March 29, 2021 16:20
'stroke-width': '70px',
fill: 'none'
},
'g[data-mml-node="mtable"] > rect[data-frame]': {
'g[data-mml-node="mtable"] > rect[data-frame], svg[data-table] > g > rect[data-frame]': {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This reminds me that I had wanted to ask about the three different kinds of "names" to build selectors - data-mml-node, data-frame/table, mjx-dashed. When converting serve-side, try to reduce some of that attribute markup (since I get even more via SRE), and these different attributes being combined for styling make it a bit hard to maintain.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what you are asking to be done. All three are important to the way the CSS works, and can't be removed. So I don't really know what you want to be different.

The data-mml-node identifies the kind of node that is being implemented by that particular g, and lets you select on the kind of MathML node there is, as is being done here. The data-frame identifies the role of the rect element (since there are other kinds of rectangles, e.g., hit-boxes for anchors). The mjx-dashed is a class assigned to line elements to set the kind of rendering to use (solid/dashed/dotted). That could have been done via a data- property, I suppose, like data-style="dashed" or some such.

Classes could have been used for the first two as well, but when we did that in v2, that lead to more instances of CSS bleed-through to the MathJax output, sO i've tried to use data attributes as being less likely to conflict with other uses. Even with classes, that doesn't reduce the CSS very much. This would still be

g.mjx-mtable > rect.mjx-frame, svg.mjx-table > g > rect.mjx-frame

so I'm not sure if that is any better.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the response!

I'm not sure what you are asking to be done.

I was asking to figure out why there isn't a single (and specific) pattern used to handle CSS based styling. Classes are the natural choice but I understand the reason for not using them (though IIRC the main problem where the more generic .mo .mi etc class names of the HTML-CSS output).

I also wondered why there are combined selectors (in the SVG output). They don't seem to be necessary or particularly useful. For example, the line I highlighted adds another combined selector but [data-frame] is not used anywhere else (afaict).

Anyway, I decided to rewrite the styles on my end. It's a bit of a maintenance headache but it allows me to be more consistent (and easier to integrate it into the surrounding design when needed). Maybe that's useful feedback.

Copy link
Contributor

@pkra pkra Mar 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They don't seem to be necessary or particularly useful.

To correct myself, there's one I know is necessary - the g[data-mml-node="mtable"] > g > svg {overflow: visible;} part which is necessary since stretchy constructions need overflow:hidden on their nested svgs (which I of course found out the hard way). I feel like that rule could benefit from a comment in the source.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I think I understand now. You are saying that since data-frame is not used anywhere else, that simply using rect[data-frame] as the selector is sufficient. Fair enough.

I have been trying to tie the selectors to the MathML node types in general and to use > to restrict the scope to the top level when possible. This is to try to prevent the CSS from applying to other content farther down in the MathML content. Since that can include arbitrary SVG (from <semantics> elements, for example) and HTML (once we allow that in the MathML <mtext> nodes), there is no telling what the content would contain, and so being as specific in the selectors here makes less likely to affect that type of content. Further, if we just used rect[data-frame], then that would apply anywhere in the page, not just inside MathJax, so the CSS could bleed outward to other content on the page. That is why I have tried to be painfully explicit about where they apply.

I guess using data-mjx- rather than just data- would alleviate some of that, or using a mjx-container prefix on everything.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the response and sorry for turning this PR into a random conversation thread. It might be an interesting conversation to have with others developers using MathJax in more unusual / custom circumstances.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it would be good to get some other use cases. I'm not sure how to make that happen. I suppose now that v3 has been out for a while and others are doing some development with it, the developer google forum might produce more results than in the past. Or we could activate discussions on GitHub (and deprecate the developer forum).

Copy link
Member

@zorkow zorkow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some typos.

@dpvc dpvc merged commit 87c36dd into develop Apr 14, 2021
@dpvc dpvc deleted the issue2601 branch April 14, 2021 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants