Page MenuHomePhabricator

Remove "^" (Prev) and "V" (Next) actions on Differential inline comments

Authored by epriestley on May 16 2017, 4:34 PM.
Referenced Files
F13253811: D17908.id43081.diff
Sat, May 25, 2:57 AM
F13252287: D17908.diff
Sat, May 25, 1:15 AM
F13221842: D17908.id43080.diff
Sun, May 19, 3:05 AM
F13215591: D17908.id43081.diff
Fri, May 17, 6:25 PM
F13215590: D17908.id43080.diff
Fri, May 17, 6:24 PM
Fri, May 17, 6:17 PM
F13215416: D17908.diff
Fri, May 17, 5:10 PM
F13198753: D17908.id43080.diff
Mon, May 13, 9:38 AM



Ref T12616. Fixes T12715. I suspect these are very rarely used. (I think you tried to get rid of them before but I pushed back since we couldn't really offer great alternatives at the time?)

Now that the code is in a better place:

  • Click an inline's header (just the colored part) to select it with the keyboard selection cursor.
  • Click again to deselect it.
  • You can use "n" and "p" to jump to comments, so "click + n" is the same as the old "V" action.
  • This also makes it easier to swap between keyboard and mouse workflows, since you can jump into things with the keyboard at any inline.

Also, make "Reply" render more consistently.

Test Plan
  • Did all that stuff, things seemed to work OK.

Diff Detail

rP Phabricator
Lint Not Applicable
Tests Not Applicable

Event Timeline

This revision is now accepted and ready to land.May 16 2017, 4:38 PM

I have some other tentative plans here but want to prototype them so I can pretend I didn't if they turn out bad.

I could also possibly imagine that a tiny number of users find this useful on mobile but we could revisit this if we get a lot of "reviewing very large, long-standing diffs on my phone was previously wonderful but you have ruined my life" feedback.

Oh! Let me see if my crazy design ideas pan out, and we can put them back if they don't and you still miss them in a week or so? I'd want to rewrite how it works anyway, so this would be the first half of the "Fix them to work the right way" diff regardless.

Yeah, I'm really wanting to rethink how inlines are done, presented, navigated, etc. It's something we should (as a project) excel at and fuss over vs. GitHub.

(Also maybe try the "click + keyboard" workflow, not sure if that'll feel good or not. I pretty much only use the mouse wheel to navigate revisions so most of these features aren't good fits for my own workflow.)

This revision was automatically updated to reflect the committed changes.