Page MenuHomePhabricator

improve differential emails, optimizing for efficiency
Closed, ResolvedPublic

Assigned To
Authored By
scode
Feb 5 2016, 4:11 PM
Referenced Files
F1093574: Screen Shot 2016-02-05 at 8.22.32 AM.png
Feb 5 2016, 4:29 PM
F1093570: Screen Shot 2016-02-05 at 8.17.18 AM.png
Feb 5 2016, 4:29 PM
F1093562: mock_after.png
Feb 5 2016, 4:11 PM
F1093545: non_inline_comments.png
Feb 5 2016, 4:11 PM
F1093551: inline_comments.png
Feb 5 2016, 4:11 PM
F1093559: mock_before.png
Feb 5 2016, 4:11 PM
F1093547: diff_creation.png
Feb 5 2016, 4:11 PM

Description

Hi,

In our environment, the typical developer receives many incoming code changes or updates to existing
ones each day. You might be off in a meeting and lunch for a couple of hour, and come back with a
backlog of 15 updates in your email inbox.

For this reason, it is important for a subset of our user population that emails are highly
efficient to take in - you want to get to the point of deciding whether to open it up in a browser
or just archive (gmail term) it, quickly, and without frustration. We want to optimize for easy and
efficiency of triaging each incoming email, rather than ignoring them.

We are currently using Review Board (just to let you know what the contrast is). Its emails are
sub-optimal for a variety of reasons, including lack of important information, and necessitating
scrolling to get down to the relevant data (particularly annoying on a phone).

However, for the common case of reading an RB email in your gmail on a laptop, it's very very quick
to hit "o, space, space, space" (for those with keyboard shortcuts) to get down to the bottom - and
the visual layout/coloring means it's very easy to spot where relevant information (someone's
comment) begins.

Phabricator emails are different. On the good side, they provide more information. However they
(subjectively?) don't do a good job of isolating the absolutely most important information from all
the rest. There is a lot of "noise" on the screen. As someone used to the "o, space, space, space"
process of quickly triaging an RB email in gmail, I feel like I have to actually read, rather than just
quickly scan, actively looking for the data that I'm interested in. A few specific points follow.

The order of the information presented does not seem optimized for "most important first"; at least
not for our situation. Meta data like repository, branch names etc are almost never something the user
cares about and should come after things like comments and diffs.

gmail often folds away the repetitious part of the email as well, meaning that you often end up
in a situation where you have the email open but there is literally no visible link to take you
to the differential, because that part of the email was folded away.

There is not a lot of visual hinting to the structure of the email (even with HTML enabled). For
example, there is no color coding to differentiate quoted text from a comment in response to
the quoted text, and diffs don't stand out as much from the boilerplate surrounding them as
they could, and even just the summary of a revision when created is hard to spot and comes after
boiler plate.

The subject lines are too verbose; too much screen real estate is eaten up by boilerplate. Instead of:

[Differential] [Request, 4 lines] D69: [phabricator] important stuff

it should probably be something like:

[Diff,D69] important stuff

The biggest exception to this is currently the way non-inline comments are displayed,
which is notably a lot better than everything else I've seen so far.

Here's an example non-inline comment, which allows you to very naturally and quickly
differentiate between quoted text and the new comment you're getting the email for:

non_inline_comments.png (764×1 px, 155 KB)

In contrast, we have the creation emails:

diff_creation.png (1×1 px, 213 KB)

And in particular the inline comments which almost feel hidden in noise:

inline_comments.png (594×1 px, 123 KB)

We don't have a concrete "here is how we think it should look", but hopefully the above communicates the gist
of what we are hoping for.

To help out a little bit, I spent a small amount of time during a crappy mockup (because I suck at visual stuff, so
bear with me), with a before/after, just to demonstrate the difference of just some filtering, re-ordering, and
visual hinting. The "after" is *not* to be construed as a suggestion for how it should actually look.

Before:

mock_before.png (910×1 px, 168 KB)

After:

mock_after.png (908×1 px, 120 KB)

Any thoughts in general on where you are wanting to go with email, and does the above seem like
reasonable desires?

Event Timeline

scode added a project: Restricted Project.Feb 5 2016, 4:28 PM
scode moved this task from Restricted Project Column to Restricted Project Column on the Restricted Project board.

Couple of quick basics for easily addressable / surface issues:

  • You can remove or change the prefix with metamta.differential.subject-prefix.
  • You can remove the [Action] particle with metamta.vary-subjects.
  • You may be able to reorder or disable some fields with differential.fields, although this applies to multiple views (you can't currently disable a field just for email). T6030 may improve this to some degree.
  • The expectation is that new requests will use the "Summary" field and possibly the "Test Plan" field. When they do, the initial email should look more like this, with the description as the most prominent content block, similar to followup emails with replies. Are you not planning to use the "Summary" field, or was this just a test, or maybe something else?

Screen Shot 2016-02-05 at 8.17.18 AM.png (682×822 px, 139 KB)

  • The default mode for inline comments does not show context. The product goal here is aligned with your stated goals: make it easy to scan the email and quickly make a decision about whether you need to follow up (e.g., is this an urgent issue, or a minor suggestion). In this default mode, inlines are easy to scan:

Screen Shot 2016-02-05 at 8.22.32 AM.png (486×820 px, 87 KB)

  • Broadly, our product intent here is that users should not be performing code review from their email client, which is why context and diffs are off by default. Instead, we expect users to quickly scan the summary, comments, etc., and use that to make a decision about whether they need to click through. Clicking through should be relatively lightweight, and the revision should render reasonably on phones (note that we've made some improvements here recently if you're a bit behind).

So I think we're overall trying to accomplish the same goals, but it looks like you've explicitly enabled several options which I think fight against those goals. For example, my expectation is that an "ignore / review" decision is almost never based on the context of an inline comment (vs the content of the comment) or the actual changes in a diff (vs the summary/discussion), which is why those are not included by default. Can you shed some more light on how you're balancing concerns here?

Broadly, these emails can certainly be improved (see T6600; see also some outstanding known issues like T6576, T7031, T7030, although I think those are non-impactful). The "with context" inlines and inline diffs are particularly bad, but these features are off by default and I believe they generally run counter to the goals of the mail and do not expect most installs to enable them (they exist for a variety of historical reasons, and because some installs have special needs), so they've been fairly far down the priority list.

eadler moved this task from Restricted Project Column to Restricted Project Column on the Restricted Project board.Feb 8 2016, 5:39 PM

@epriestley Thanks! Sorry about our screw-up with the settings. Looks like we had an internal communication problem. I'll follow up and make sure we're exhausting what settings we have available to us (starting with default options) and return to this and the tickets you linked after that.

Cool, sounds good. There's definitely room for improvement here and we're open to making changes if it turns out you do actually have a few competing use cases to balance, but I suspect you can make reasonable headway on at least some of this by revisiting Config with an eye to dialing settings back toward optimizing the "ignore / review" decision.

One possible example is that Phabricator might not normally be accessible from phones because users need to be VPN'd, which would make clicking through hard/impossible. This might be a good motivating use case for improving the rendering of diffs and inline comments. (See also differential.enable-email-accept in Config.)

eadler moved this task from Restricted Project Column to Restricted Project Column on the Restricted Project board.Feb 18 2016, 6:29 PM
eadler moved this task from Restricted Project Column to Restricted Project Column on the Restricted Project board.Jul 4 2016, 9:06 PM
epriestley claimed this task.

(I think everything actionable here was addressed, or survives in followups.)