Skip to content

KaTeX (3/n): Support negative margins for spans #1559

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

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

rajveermalviya
Copy link
Member

Stacked on top of #1452.

Related: #46

@gnprice
Copy link
Member

gnprice commented Jun 10, 2025

@rajveermalviya Copying here for visibility what you told me on our call today: the reason this is a draft (not ready to merge) is that you're still working on the widget test. Apart from that, you consider it ready to review.

@gnprice gnprice added the maintainer review PR ready for review by Zulip maintainers label Jun 10, 2025
@rajveermalviya rajveermalviya marked this pull request as ready for review June 11, 2025 17:34
rajveermalviya and others added 14 commits June 11, 2025 23:59
This will prevent string interpolation being evaluated during
release build. Especially useful in later commit where it becomes
more expensive.
This applies the correct font scaling if the KaTeX content is
inside a header.
And rename previous type to KatexSpanNode, also while making it a
subtype of KatexNode.
In KaTeX HTML it is used to set the baseline of the content in a
span, so handle it separately here.
And inline the behaviour for `inline: false` in MathBlock widget.
Implement handling most common types of `vlist` spans.
Negative margin spans on web render to the offset being applied
to the specific span and all the adjacent spans, so mimic the
same behaviour here.
Copy link
Member

@gnprice gnprice left a comment

Choose a reason for hiding this comment

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

Thanks! Here's small comments from a partial skim.

Comment on lines +39 to +40
// Like [RenderPadding] but supports negative values.
class RenderNegativePadding extends RenderShiftedBox {
Copy link
Member

Choose a reason for hiding this comment

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

Let's leave a TODO tracking our intention to get a version of this upstream:

Suggested change
// Like [RenderPadding] but supports negative values.
class RenderNegativePadding extends RenderShiftedBox {
// Like [RenderPadding] but supports negative values.
// TODO(upstream): give Padding an option to accept negative padding (at cost of hit-testing not working)
class RenderNegativePadding extends RenderShiftedBox {

Comment on lines +200 to +201
extension on EdgeInsetsGeometry {
bool get isNegative => !isNonNegative;
Copy link
Member

Choose a reason for hiding this comment

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

I think this is clearer (and hardly much longer) if just inlined.

In particular there's some ambiguity in that "is negative" sounds like it might require all the sides to be negative. So being more transparent, with one fewer layer of indirection, helps mitigate that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintainer review PR ready for review by Zulip maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants