Page MenuHomePhabricator

Pick context windows for inlines in a slightly smarter way
ClosedPublic

Authored by epriestley on May 5 2016, 6:09 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Jun 26, 9:52 AM
Unknown Object (File)
Sat, Jun 25, 8:39 AM
Unknown Object (File)
Fri, Jun 24, 5:12 AM
Unknown Object (File)
Sun, Jun 19, 10:22 AM
Unknown Object (File)
Sun, Jun 19, 4:53 AM
Unknown Object (File)
Tue, Jun 14, 2:40 AM
Unknown Object (File)
Sun, Jun 12, 2:35 PM
Unknown Object (File)
Sun, Jun 12, 10:10 AM
Subscribers
None

Details

Summary

Ref T10694. This mostly prevents us from having a degenerate case if someone leaves a 200-line inline.

  • For one-line inlines, show 1 line of context above and below (3 lines total).
  • For 3+ line inlines, show just the inline.
  • For 7+ line inlines, show only the first part.
Test Plan

Made a bunch of weird long/short/different-sized comments, saw reasonble-appearing context in text and HTML mail output.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

epriestley retitled this revision from to Pick context windows for inlines in a slightly smarter way.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: chad.
chad edited edge metadata.
This revision is now accepted and ready to land.May 5 2016, 6:12 PM
This revision was automatically updated to reflect the committed changes.

.oO

src/applications/differential/mail/DifferentialInlineCommentMailView.php
289

little inline

297–299

Medium inline.

301–320

BIG INLINE!

more icons, more roundeder corners, more gradients needed

This is what I'm seeing, which doesn't look terrible:

Screen Shot 2016-05-05 at 11.26.03 AM.png (753×942 px, 114 KB)

Are you seeing something way worse?

oic

I'm not sure we can do icons in mail

But feel free to graident/round everything

Well, doesn't look totally terrible to my untrained engineer-eyes, at least.

let me get kiddo to nap then i'll photomock something

Sounds good.

We can probably use actual unicode character "icons", but it looks like data:.. URIs don't work in a ton of clients and I don't think we can get FontAwesome working in Gmail from googling. We could serve actual images off Phabricator, but I suspect we may be dissatisfied with the results/complexity/support load that generates.

i was joking about icons, I know you love them

Did you see how many icons I added to repositories? Whole thing is basically just different icons now.

src/applications/differential/mail/DifferentialInlineCommentMailView.php
297–299

This is a reply with some inline styles.

301–320

This is a reply with a quote block containing a codeblock, which might be probably somewhat difficult to parse visually in mail?

HAHAHAHAHA();