Page MenuHomePhabricator

Add ability to quote, edit, view raw, view history, and remove inline comments
Open, Needs TriagePublic


Some inline comments can be quite in-depth if they raise a design issue. (My team is not based in the same location, so these discussions don't always arise face-to-face, but are hashed out in Differential, which we use somewhat as a "Request for Comments" mechanism as well as a review tool.) As such, we often want to quote part of a lengthy inline comment in a reply. At the moment this is tedious. We have to use copy/paste, which doesn't preserve any of the Remarkup. What's worse, you can only copy/paste from where the inline comment appears in the transaction on the timeline, not where it appears within the code/diff (for some reason, text selection simply doesn't work there, at least using my FF Mac browser).

A quote facility for inline comments would solve this. Perhaps a quotation mark icon beside the reply icon which opens a reply prefilled with the quoted inline comment.

A somewhat-related issue is the ability to edit inline comments. From time to time I have wanted to do this.

Perhaps the most commonly used actions (reply, previous, next) could live in the comment header, but less frequent ones move to a menu like comments on the timeline (quote, edit, hide, view raw?).

Event Timeline

Yeah, I think this is largely an issue of how much space we have in the UI.

There's no technical or product reason that we don't support Edit, View Raw and Quote (specifically, "Reply, Starting With a Quote" like you describe) except that putting three more buttons in the headers probably runs us into trouble, especially on mobile.

We also need "View History" as soon as we have "Edit", and probably want "Remove" for parity with other applications.

I'm hesitant to put "Hide" in a dropdown since I think users who do use it use it benefit from easy access. I don't personally use it very often, but when I do it's often because I want to hide a bunch of things and I'll go use it on a lot of inlines at once.

I wonder if we could get away with putting the previous/next buttons in a dropdown if we improved keyboard navigation a bit? You could potentially use the dropdown to learn about the next/prev keystrokes and then use them from then on, so it wouldn't be as important that the actions weren't easy to get to. I think we'd have to make mouse and keyboard interactions blend together better, though -- right now, there's no way to set the next/prev cursor to an inline you're looking at. If you could click the inline, then press "n", that might let us bury the buttons.

epriestley renamed this task from Add ability to quote inline comments to Add ability to quote, edit, view raw, view history, and remove inline comments.Jul 31 2016, 7:08 PM
eadler added a project: Restricted Project.Aug 7 2016, 7:59 PM

I'm not sure if D17908 is going to stick, but if it does we more reasonably have room to add a caret menu here. I'm not planning to necessarily pursue this in the current iteration, but I believe there are no longer any meaningful technical blockers, just some product questions which will more or less resolve themselves if everything sticks.

I think if we combine dropdown with keyboard hints, it'd be a good middle ground.

That is, we show the keyboard shortcut on the dropdown along with the action.

If nothing changes, I was thinking of leaving these top-level:

  • Done
  • Edit
  • Reply
  • Hide
  • (New Dropdown)

...and putting these in the new dropdown:

  • View Remarkup
  • Edit History
  • Remove Comment

We could fiddle with that, but my gut is that "Done", "Edit", "Reply" and "Hide" are still important/common enough to leave at top level. We could also put "Quote" into the dropdown, I suppose.

I'm hesitant to put, e.g., "Reply" into the dropdown even if it says "Reply (by the way, you can press r to reply)" because you have to select the comment first, which is extra work and I think not tremendously intuitive. Maybe we're moving in a better direction than I think with the new keyboard selection state, but the keyboard shortcuts still feel like an extreme power-user feature to me even though I've tried to blunt the edge a little.

(I normally think of myself as like a 95th percentile power user, and don't use keyboard shortcuts at all in Differential today, and suspect I still won't after these changes. However, I'm also an elite mouse power user, per J769.)

I'm also toying with the idea of "Shift+Click" on "Reply" to "Reply-with-Quote" but the only way to discover that would be to read the documentation.

I do think "Quote" needs to be somewhere (it wasn't listed at all in T11401#224171, though you mentioned it). Shift-click on reply could also be supported, but such a power user feature is far less important than an easily discoverable function. I believe that I personally would use "Quote" more often than "Edit", but honestly would be happy wherever this were put. I'll probably upgrade my Phab instance and start using the "R" shortcut next week either way.

FWIW, I use the keyboard navigation in Differential a lot, i.e. almost every diff. I am visually impaired so there's a large overhead to using the mouse (finding where the cursor is on the screen, peering closely to see if I'm pointing at the right place, etc.). It is easier to discover keyboard shortcuts, and you are also reminded of them, when they are listed in a menu, or appear as tool tips on hover.