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

Branch

mono2

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 12177
Build 15369: Run Core Tests
Build 15368: Run Core Tests
Build 15367: arc lint + arc unit

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.