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

Prevent disappearing fraction lines. #931

Merged
merged 2 commits into from Oct 14, 2017

Conversation

Projects
None yet
3 participants
@ronkok
Collaborator

ronkok commented Oct 12, 2017

Set a CSS min-height that will ensure that fraction lines will display at least one screen pixel.

Addresses issues #824 and #916.

Prevent disappearing fraction lines.
Set a CSS min-height that will ensure that fraction lines will display at least one screen pixel.

Addresses issues #824 and #916.
@kevinbarabash

This comment has been minimized.

Show comment
Hide comment
@kevinbarabash

kevinbarabash Oct 12, 2017

Member

@ronkok cool! When zoomed out the some layouts may look a little more cramped. Could you post some screenshots to show what that looks like at small font sizes and/or zoomed out?

Member

kevinbarabash commented Oct 12, 2017

@ronkok cool! When zoomed out the some layouts may look a little more cramped. Could you post some screenshots to show what that looks like at small font sizes and/or zoomed out?

@ronkok

This comment has been minimized.

Show comment
Hide comment
@ronkok

ronkok Oct 12, 2017

Collaborator

I'll have some screenshots ready tomorrow. Also, I should point out that IE does not support dppx. So IE's min-height will always be 1px. I'm not troubled by that. I expect few IE users have a high resolution display.

Collaborator

ronkok commented Oct 12, 2017

I'll have some screenshots ready tomorrow. Also, I should point out that IE does not support dppx. So IE's min-height will always be 1px. I'm not troubled by that. I expect few IE users have a high resolution display.

@kevinbarabash

This comment has been minimized.

Show comment
Hide comment
@kevinbarabash

kevinbarabash Oct 12, 2017

Member

Also, I should point out that IE does not support dppx.

Do you know if Edge supports dppx?

Member

kevinbarabash commented Oct 12, 2017

Also, I should point out that IE does not support dppx.

Do you know if Edge supports dppx?

@ronkok

This comment has been minimized.

Show comment
Hide comment
@ronkok

ronkok Oct 13, 2017

Collaborator

Yes, Edge does support dppx.

Collaborator

ronkok commented Oct 13, 2017

Yes, Edge does support dppx.

Change from dppx to dpi
Change the break point spec from 2dppx to 192dpi. They mean the same thing. I'm making the change because IE supports dpi but not dppx.
@ronkok

This comment has been minimized.

Show comment
Hide comment
@ronkok

ronkok Oct 14, 2017

Collaborator

This post shows some screen shots to demonstrate the effect of this PR. The image in the screen shots is a series of fractions, written with font-size from 16px down to 7px in 1px increments. All of the screenshots were taken with the browser set to 100% magnification. Then each shot was enlarged to 400% in Microsoft Paint.

The first two screen shots are done to show the efficacy of this PR. The first screen shot shows the current KaTeX master in Chrome, Firefox, and IE 11 on a screen with resolution = 166 dpi. Notice the missing fraction bars in Chrome.
master-166dpi

The second screen shot is on the same browsers, on the same screen, but includes this PR. All the bars are present. Hurray!
pr-166dpi

The next two screen shots are done to investigate whether this PR creates frac bars that are too thick on a higher resolution screen. Remember, the goal of this PR is ensure at least one screen pixel, but other than that, leave all frac-bar thickness determination to the 0.04 em calculation in KaTeX master.

The next (third) screen shot shows the current KaTeX master in Chrome, Firefox, Opera, and Edge on a screen with resolution = 272 dpi. It all looks good.
master-277dpi
The final screen shot is on the same screen. It shows the result of this PR. It all looks good except that some Edge lines have thickened to two screen pixels.
pr-277dpi
So we have a tradeoff of some thickened Edge lines vs. some missing Chrome lines. I think this PR is a net gain.

I've also put up a test page so others can look at the results. I'm particularly interested in Safari.

Collaborator

ronkok commented Oct 14, 2017

This post shows some screen shots to demonstrate the effect of this PR. The image in the screen shots is a series of fractions, written with font-size from 16px down to 7px in 1px increments. All of the screenshots were taken with the browser set to 100% magnification. Then each shot was enlarged to 400% in Microsoft Paint.

The first two screen shots are done to show the efficacy of this PR. The first screen shot shows the current KaTeX master in Chrome, Firefox, and IE 11 on a screen with resolution = 166 dpi. Notice the missing fraction bars in Chrome.
master-166dpi

The second screen shot is on the same browsers, on the same screen, but includes this PR. All the bars are present. Hurray!
pr-166dpi

The next two screen shots are done to investigate whether this PR creates frac bars that are too thick on a higher resolution screen. Remember, the goal of this PR is ensure at least one screen pixel, but other than that, leave all frac-bar thickness determination to the 0.04 em calculation in KaTeX master.

The next (third) screen shot shows the current KaTeX master in Chrome, Firefox, Opera, and Edge on a screen with resolution = 272 dpi. It all looks good.
master-277dpi
The final screen shot is on the same screen. It shows the result of this PR. It all looks good except that some Edge lines have thickened to two screen pixels.
pr-277dpi
So we have a tradeoff of some thickened Edge lines vs. some missing Chrome lines. I think this PR is a net gain.

I've also put up a test page so others can look at the results. I'm particularly interested in Safari.

@kevinbarabash

This comment has been minimized.

Show comment
Hide comment
@kevinbarabash

kevinbarabash Oct 14, 2017

Member

@ronkok thanks for all the screenshots comparing all the browsers before/after. I agree, this is definitely a net win.

Member

kevinbarabash commented Oct 14, 2017

@ronkok thanks for all the screenshots comparing all the browsers before/after. I agree, this is definitely a net win.

@kevinbarabash

LGTM. A very simple solution to a problem affecting many. Thanks.

// Modify min-height for retina displays. Don't force two screen pixels.
@media screen and (-webkit-min-device-pixel-ratio: 2.0),
screen and (min-resolution: 192dpi) {

This comment has been minimized.

@kevinbarabash

kevinbarabash Oct 14, 2017

Member

I'm going to merge this, but I'm curious about the choice of dpi... is it b/c Window's default 1x resolution is 96dpi?

@kevinbarabash

kevinbarabash Oct 14, 2017

Member

I'm going to merge this, but I'm curious about the choice of dpi... is it b/c Window's default 1x resolution is 96dpi?

This comment has been minimized.

@ronkok

ronkok Oct 14, 2017

Collaborator

It's not just a Windows thing. According to this post,192 dpi and 2 dppx are the same thing. Now, I know that a blog post from W3C is not as authoritative as a W3C standard, but I still take it as pretty good information.

And by using dpi, we get support from IE, which does not support dppx.

@ronkok

ronkok Oct 14, 2017

Collaborator

It's not just a Windows thing. According to this post,192 dpi and 2 dppx are the same thing. Now, I know that a blog post from W3C is not as authoritative as a W3C standard, but I still take it as pretty good information.

And by using dpi, we get support from IE, which does not support dppx.

This comment has been minimized.

@kevinbarabash

kevinbarabash Oct 14, 2017

Member

Interesting. Hmm... I wonder if we can we can supply both dppx values and dpi values for min-resolution in that order. Not sure if there's a browser out there that supports dppx but doesn't do the correct thing with dpi as described in the "caveats" section of the blog post.

@kevinbarabash

kevinbarabash Oct 14, 2017

Member

Interesting. Hmm... I wonder if we can we can supply both dppx values and dpi values for min-resolution in that order. Not sure if there's a browser out there that supports dppx but doesn't do the correct thing with dpi as described in the "caveats" section of the blog post.

@kevinbarabash kevinbarabash merged commit 50d120d into Khan:master Oct 14, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@edemaine

This comment has been minimized.

Show comment
Hide comment
@edemaine

edemaine Oct 15, 2017

Collaborator

Great idea @ronkok!

Given that this was just an issue on Chrome (and maybe recent Chrome?), I wonder if we should submit an upstream bug report too?

Collaborator

edemaine commented Oct 15, 2017

Great idea @ronkok!

Given that this was just an issue on Chrome (and maybe recent Chrome?), I wonder if we should submit an upstream bug report too?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment