Can u do dis 4 me. Pls. :3
Main reasoning here is "done" comments should be more of a greyed out "complete" look to them, and un-done should still be yellow so it's easy to spot things you haven't addressed.
chad | |
Mar 25 2015, 5:29 PM |
F351237: pasted_file | |
Mar 26 2015, 4:42 PM |
F350488: pasted_file | |
Mar 25 2015, 5:37 PM |
Can u do dis 4 me. Pls. :3
Main reasoning here is "done" comments should be more of a greyed out "complete" look to them, and un-done should still be yellow so it's easy to spot things you haven't addressed.
rP Phabricator | |||
D12171 Revamp inline commenting UI | |||
D12160 | rP1fd163d0976c Mostly provide CSS for "done" states |
Yeah -- we actually have four states:
My thinking was maybe to sync the overall color with whether the checkbox is checked or not, and then show some local style on the checkbox to hint that it's unpublished. For example, crudely:
I also suspect we want replies to count as giving inlines a "complete" look.
Philosophically, I also want to be careful about making these states too bold, since I don't want users to feel like they're obligated to go check things off. Making the states really extreme could make checking boxes feel more mandatory.
Anyway, I'll give you some CSS classes and we can go from there.
I have this little guy for 'unsubmitted', easy to show and hide with CSS rules. Might have to dial it up, but we'll see.
Oh, the actual checkmark itself can be unsubmitted though. Presumably that should be communicated differently than the comment as a whole being a draft.
That is, if you make an inline comment and I check it off as done, no one but me can see that checkmark until I comment on the revision, update it with a new diff, or take another major action. All of the stuff I checked off publishes all at once at that time, as a single "epriestley marked 13 inlines done." event. Until then, when you and I look at the same inline, you'll see it unchecked and I'll see it checked.
Is there a good reason to have each inline-comment in a new <tr> vs. just stacking them in the same cell?
Mostly, it makes the code simpler because it doesn't have to check whether it needs to add a new row or not when adding comments, and doesn't have to check whether it needs to remove a row or not when removing comments.
It also makes layout simpler when there are inlines on both the left and right side of a diff.
I'm going to get this to 95% or so and let you commandeer it for the JS edge cases we're not getting currently.
Right now I think its:
I would like to see an avatar in there maybe just if comment is from the author. I want to denote author in some regards, but not sure if it should be text or just a slight visual cue.
Unpublished "Done" is should be the same, but is broken in JS.
This state should not be possible: you can not click "Done" on an unpublished draft comment. Am I misunderstanding?
Sorry, that clicking "Done" on a Published comment should grey out the comment box, and display the red "Unpublished Draft" tag
So "Unpublished Draft" means both:
That seems really confusing to me, especially because I don't think of the checkmark as a "draft". Is there a specific reason you don't want to use a different indicator for "unpublished comment" vs "unpublished checkmark"?
The indicator is just a red bubble with text. If you want to change the text in the bubble to be more concise, thats fine with me. It hasn't felt weird in my sandbox.