close
Skip to content

Change frac-line from border to full span#976

Merged
k4b7 merged 8 commits into
KaTeX:masterfrom
ronkok:fraclinespan
Nov 24, 2017
Merged

Change frac-line from border to full span#976
k4b7 merged 8 commits into
KaTeX:masterfrom
ronkok:fraclinespan

Conversation

@ronkok
Copy link
Copy Markdown
Collaborator

@ronkok ronkok commented Nov 20, 2017

Change frac-line and vertical-separator. Instead of using span borders to render the lines, fill an entire span using a box-shadow SVG.

This change will enable more dependable engagement of the min-height CSS.

Change `frac-line`  and `vertical-separator`.  Instead of using span borders, fill entire span using a `box-shadow`.

This change will enable more dependable engagement of the `min-height` CSS.
@marcianx
Copy link
Copy Markdown
Collaborator

FYI: I'd be wary of using box-shadow for anything since in Windows high contrast mode, browsers remove all box shadows (and also background colors/images). I've totally been bitten by this when working on making accessible web UIs.

@ronkok
Copy link
Copy Markdown
Collaborator Author

ronkok commented Nov 20, 2017

Pity. I already knew that background colors were a bad idea when printing. I suppose I could use an SVG image. Well, this will take a little longer.

@edemaine
Copy link
Copy Markdown
Member

Is this true even if the settings are marked !important? (relevant to both backgrounds and shadow boxes) It seems a shame to have to use such advanced features for something as simple as a fraction... though I'm not coming up with an alternative?

@marcianx
Copy link
Copy Markdown
Collaborator

!¡mportant is specifically designed to override CSS precedence. I'm not aware of it offering any other use.

@edemaine
Copy link
Copy Markdown
Member

@marcianx It's at least sometimes relevant, but maybe only in ms-specific rules... See #716 (comment)

@ronkok
Copy link
Copy Markdown
Collaborator Author

ronkok commented Nov 20, 2017

Interesting.

I want to avoid background-color, even if there is a way to avoid the high contrast problem. When printing, the default setting is to omit background colors.

But regarding box-shadow, as @edemaine points out, the KaTeX CSS already includes

.katex * {
  -ms-high-contrast-adjust: none !important;
}

I just finished writing the code for a frac-line via inline SVG image. The code works fine, but I have not made a commit. First, I'll check if box-shadow works when high-contrast is set and also works when printing.

@ronkok
Copy link
Copy Markdown
Collaborator Author

ronkok commented Nov 20, 2017

High contrast results are in. The image on the left is the box-shadow method. On the right is the inline SVG image.

box shadow
SVG is the clear winner. I'll turn in a commit.

Thank you to @marcianx for the warning about high contrast.

@ronkok
Copy link
Copy Markdown
Collaborator Author

ronkok commented Nov 20, 2017

Well, now there’s a working PR and, thanks to @marcianx, we have overcome one hurdle. At this point, I am the wrong person to do any further detailed testing on this work. I have not been able to reproduce the problem on my own screen and I have never mastered Docker for the Screenshotter tests. Any help would be appreciated.

@trimbonz
Copy link
Copy Markdown

Both the box-shadow and SVG methods in this PR fix the issue for my old Android webView clients, that would previously not render any fraction lines up to and including API 19 (KitKat). Previously I had to tweak the line thicknesses manually to make fraction lines appear, but they all show up without further intervention after I started using the fraclinespan branch.

@ronkok
Copy link
Copy Markdown
Collaborator Author

ronkok commented Nov 22, 2017

@trimbonz Cool! Thank you for the test and for the report.

@k4b7
Copy link
Copy Markdown
Member

k4b7 commented Nov 24, 2017

@ronkok I can regenerate the screenshots.

Comment thread src/stretchy.js
["preserveAspectRatio", "xMinYMin slice"],
];
const svg = new domTree.svgNode([pathNode], attributes);
return buildCommon.makeSpan([className, "hide-tail"], [svg], options);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

How is the height (or width) set for the ruleSpan?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The hide-tail class contains the CSS overflow: hidden;. So the height that gets rendered is the line.style.height set by buildHTML.makeLineSpan().

Class frac-line has always contained the CSS width: 100%;, so the width is being set the way it always has been set.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Clarification: My response above pertained to \frac, \overline, and \underline. I did not change \rule in this PR, so it is still using a top-border to render a line.

Comment thread static/katex.less
@media screen and (-webkit-min-device-pixel-ratio: 2.0),
screen and (min-resolution: 192dpi) {
min-width: 0.5px;
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should .frac-line have its min-height set in a similar fashion?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

frac-line already has that min-height set. That was the purpose of PR #931.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for refreshing my memory. 😅

Copy link
Copy Markdown
Member

@k4b7 k4b7 left a comment

Choose a reason for hiding this comment

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

LGTM

@k4b7
Copy link
Copy Markdown
Member

k4b7 commented Nov 24, 2017

@ronkok thanks for making these changes.

@k4b7 k4b7 merged commit b2698d3 into KaTeX:master Nov 24, 2017
@ronkok ronkok deleted the fraclinespan branch November 24, 2017 16:45
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.

5 participants