Page MenuHomePhabricator

Add links and diffs for text block edits to mail
ClosedPublic

Authored by epriestley on Jun 6 2016, 10:36 PM.
Tags
None
Referenced Files
F12837752: D16063.id.diff
Thu, Mar 28, 5:26 PM
Unknown Object (File)
Fri, Mar 22, 7:23 PM
Unknown Object (File)
Thu, Mar 21, 10:26 PM
Unknown Object (File)
Thu, Mar 21, 9:53 PM
Unknown Object (File)
Thu, Mar 21, 9:50 PM
Unknown Object (File)
Thu, Mar 21, 8:28 PM
Unknown Object (File)
Mon, Mar 18, 7:22 PM
Unknown Object (File)
Mon, Mar 4, 7:56 PM
Subscribers
None

Details

Summary

Ref T7643.

  • When a transaction edits a text block, add a link to the changes (for HTML mail).
  • Also, inline the changes in the mail (for HTML mail).
  • Do nothing for text mail since I don't think we really have room? And I don't know how we can make the diff look any good.
Test Plan

Edited a task description, generated mail, examined mail.

  • It contained a link leading to a prose diff.
  • It had a more-or-less reasonable inline text diff.

Diff Detail

Repository
rP Phabricator
Branch
prose5
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 12524
Build 15882: Run Core Tests
Build 15881: arc lint + arc unit

Event Timeline

epriestley retitled this revision from to Add links and diffs for text block edits to mail.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: chad.

Here's a maybe-workable way to get this operational for text diffs -- we can use unicode combining marks to "style" the text:

The dog said "m̶e̶o̶w̶ w̳o̳o̳f̳". The d̶u̶c̶k̶ c̳o̳w̳ said "moo".

At least on my system, that looks like an illegible mess, though. We could fiddle with different characters but I suspect this isn't too promising as an avenue.

Here's what I see, seems pretty much impossible to read:

Screen Shot 2016-06-06 at 3.45.31 PM.png (66×372 px, 14 KB)

These are more readable but it's impossible to tell which is "-" and which is "+":

  • The dog said m͓e͓o͓w͓ w̟o̟o̟f̟.

These are a little better-ish? Maybe?

  • The dog said m̌ěǒw̌ w̭o̭o̭f̭.

I also suspect most users opting out of HTML email are using some 1970-era terminal client that they compiled from C by hand and won't have new-fangled unicode.

I pushed for some feedback downstream: https://phabricator.wikimedia.org/T75851#2359538

It also might not be terrible to just dump a block of links into plain text email:

EDIT DETAILS
Description: https://dev.org.com/transaction/detail/xyz/
Ancient Lore: https://dev.org.com/transaction/detail/abc/

(Usually there's only one, but if you add 5 custom text fields it seems like putting five separate sections into the mail is a bit silly. Although maybe that's fine, too.)

I could also make the URIs very slightly prettier if we go down this route.

chad edited edge metadata.
This revision is now accepted and ready to land.Jun 7 2016, 12:05 AM
This revision was automatically updated to reflect the committed changes.