For the actual inline, here's the set of elements I'd like to try starting with:

• Username
• Comment
• Should we just get rid of the line range? It doesn't seem terribly useful, since you can just hover over the comment. Maybe a mild argument to retain it for mobile?
• Previous, Next, Edit, Delete and Reply actions
  • Unless we change up the design a lot, we're going to be tight on space -- maybe previous/next become arrows/chevrons? Maybe all of these become icons?
• Done state. Here's my v0 proposal for this, specifically:
  • This is a checkbox on the comment, probably in the upper-right position.
  • Before the comment is submitted, the comment author can edit it. This allows comment authors to submit "advisory" inlines in environments where "done" is used. For example, if you want to submit an inline like "Nice cleanup!", you can check the box and immediately submit it into a "Done" state, indicating that you don't expect the revision author to respond to it.
  • After the comment is submitted, the revision author can edit it. Other users can not edit it.
    • Clicking it puts it into a "draft complete" state which isn't visible to others.
    • Updating the revision or submitting a comment converts all "draft complete" checkmarks into "published complete" checkmarks.
• Reply state. Here's my v0 proposal for this, specifically:
  • Some kind of icon indicating that this is a reply.
  • Clicking it jumps to the comment it is a reply to.
  • Not sure if an icon indicating that the comment has been replied to is important or not.

I'd like to try solving T3669 by overkilling the draft-ness of draft comments -- instead of making them greyer with a dashed border:

• Like, make the whole thing green.
• Move "Not Submitted Yet" to an infoView style yellow subheader.
• Spell out "This draft comment has not been published yet."

If that fixes T3669, we can edge back from the brink and make it less ridiculous, but if this works it would be a better line of attack than anything in T3669 (all of which is a mess with a bunch of bad tradeoffs). I also don't think making drafts more visible is a bad thing; they're currently slightly less visible than normal comments, if anything.

That will also free up grey for use in T7447, without ambiguity between draft and ghost comments.

This doesn't deal with augmenting the timeline view, porting comments forward, or providing an alternate summary view or alternate threaded view, but I'd like to get the basic mechanical edit interactions working first. Once the mechanics work we can explore the other UI components.

I anticipate enhancing the timeline view to show reply/done status and providing a summary an additional summary/or threaded view (@chad has some threaded mocks) but I don't think they're likely to have much of an impact on the primary UI.

When user have checked "Done" checkbox in inline comment, but haven't used large commenting form at the bottom, then comment end up in fixedState = 'draft' and no indication elsewhere is found:

• after large comment form indicating, that there are drafts
• in the list the yellow speech bubble

See some discussion in T7660.

Thanks.

• the left margin of Done checkbox is incorrect, which results in left checkbox edge being cut off
• the vertical align links is incorrect

This is is on iPhone 5S in Landscape and in Portrait modes.

This feature is a big hit over here at khan academy!

There's been a feature request that the checkbox also exist at the top-of-page comments. I think the idea is that if you addressed a bunch of comments, you can just go through the top of the page going 'done done done' without having to scroll to each comment individually.

@csilvers D12171 is the next UI piece to land, once that's polished up we'll look at the comments-table

At first actual-real-use, this state feels a little weird to me (the "Not Done", specifically). Maybe we should just hide the checkbox element on drafts, now that we aren't letting you pre-check your own comments?

Easy enough to do

Easy enough to do

(At the same time, I hesitate a bit because having the icons up there be totally different for every comment is also weird.)

New inline comments are really awesome overall, though.

In T1460#104476, @epriestley wrote:

(At the same time, I hesitate a bit because having the icons up there be totally different for every comment is also weird.)

New inline comments are really awesome overall, though.

Agreed, although it seems like the blue lint advice box is missing a background color.

Mostly we're trying to convey more with less color. These new items take up more space, but color/visual impact should be roughly the same. The blue stands out quite a bit, still.

The Done state should maybe propagate to the replies of the comment?

We're liking the new feature to mark inline comments as done, however, we dislike the change that makes old comments "stick" to updated diffs. This makes it much harder to read the new updated diff when there are lots of inline comments. Any plans to improve this somehow?

See T7447.

In T1460#108778, @avivey wrote:

The Done state should maybe propagate to the replies of the comment?

Very much +1 to this.

In T1460#109400, @asteinlein wrote:

In T1460#108778, @avivey wrote:

The Done state should maybe propagate to the replies of the comment?

Very much +1 to this.

I just wanted to expand my comment on this and provide suggestions. We're seing quite a lot of "Not Done" simply when it's follow-up comment in a thread in which the main issue/comment has been solved, making this feature harder to use "reliably" to see remaining issues on the patch. In our use-cases, people sometimes just say "thanks", "didn't know that" or whatever, while in other cases provide clarifications for their thinking as follow-up comments, even though the original issue/comment has been fixed. Having they become "Not Done" just adds noise.

Rarely does a follow-up comment involve "a new issue". For that case, I suggest a reviewer should be able to re-open inline comments, for instance with having the "Done" sticker turn into "Reopen" button or some such when hovered.

Not sure if we're just weird or if this would be preferable behavior in general, though... :)

From T7447:

In T7447#112510, @jparise wrote:

I've been enjoying the combination of the "Done" flag and inherited comments, but one thing that I've found bothersome is that I need to click "Done" for each comment in a "thread" to "close out" that line of conversation (once I've actually done the requested thing).

I feel like I'm doing something wrong. Perhaps "Done" should only appear once for each group of comments? Or clicking "Done" on a comment should mark all earlier comments as done, allowing new comments to be added that are still "undone".