Page MenuHomePhabricator

Remove hard-coding of diff line height
ClosedPublic

Authored by chad on May 13 2016, 12:03 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Mar 27, 12:29 PM
Unknown Object (File)
Wed, Mar 27, 12:29 PM
Unknown Object (File)
Wed, Mar 27, 12:29 PM
Unknown Object (File)
Wed, Mar 20, 6:08 PM
Unknown Object (File)
Wed, Mar 20, 6:08 PM
Unknown Object (File)
Wed, Mar 20, 6:08 PM
Unknown Object (File)
Mon, Mar 4, 7:19 PM
Unknown Object (File)
Jan 19 2024, 9:33 PM

Details

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.48.24 PM.png (193×910 px, 40 KB)

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 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.

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

pasted_file (58×152 px, 8 KB)

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.

Current display is a little off and bugs me as well.

Screen Shot 2016-05-12 at 9.58.06 PM.png (256×314 px, 27 KB)

chad edited reviewers, added: epriestley; removed: chad.
  • Slightly better colors, spacing
epriestley edited edge metadata.

finally I can review changes in 24px / 48px impact

This revision is now accepted and ready to land.May 16 2016, 5:20 PM
This revision was automatically updated to reflect the committed changes.