Remove hard-coding of diff line height
ClosedPublic
Actions

Authored by chad on May 13 2016, 12:03 AM.

Details

Reviewers

epriestley

Maniphest Tasks

T10959: Hard-coded diff line height interacts poorly with large custom font sizes

Commits

rP03a1deba23f6: Remove hard-coding of diff line height

Summary

Fixes T10959. This is the smallest/simplest fix that I could come up with, and I wasn't able to break it. Basically, I removed "line-height" and then adjusted other rules until the defaults looked reasonable again.

Test Plan

Here's 24px / 48px impact or something like it:

Screen Shot 2016-05-12 at 4.58.00 PM.png (485×1 px, 91 KB)

Here's normal stuff working properly without weird artifacts on the highlighting:

Screen Shot 2016-05-12 at 4.58.50 PM.png (381×1 px, 69 KB)

Also tested Firefox and Chrome and got similar results.

Diff Detail

Repository

rP Phabricator

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

epriestley updated this revision to Diff 38305.May 13 2016, 12:03 AM

epriestley retitled this revision from to Remove hard-coding of diff line height.

epriestley updated this object.

epriestley edited the test plan for this revision. (Show Details)

epriestley added a reviewer: chad.

epriestley added a task: T10959: Hard-coded diff line height interacts poorly with large custom font sizes.

Herald edited edge metadata. · View Herald TranscriptMay 13 2016, 12:03 AM

Mobile/device/1-up seem OK too, from what I can tell.

theres this hanging pixel in the baseline of the darker red, I think that padding you're removing is pulling it out?

Anyways, there are a couple nits like this I'd like to polish up, I can take this over and send it back to you once I've centered it better, and get the fill correct.