Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Proposal for updating commenting area for diffs #115808

Closed
miguelsolorio opened this issue Feb 5, 2021 · 6 comments · Fixed by #157508
Closed

Proposal for updating commenting area for diffs #115808

miguelsolorio opened this issue Feb 5, 2021 · 6 comments · Fixed by #157508
Assignees
@alexr00
Labels
comments Comments Provider/Widget/Panel issues feature-request Request for new features or functionality insiders-released Patch has been released in VS Code Insiders on-testplan ux User experience issues
Milestone
August 2022

Comments

@miguelsolorio
Copy link
Contributor

miguelsolorio commented Feb 5, 2021 •
edited
Loading

I was exploring ways to improve the PR experience in VS Code and one of the ideas involved updating the glyph area for comments. Currently we show a gray bar and a unicode + and ♦ that don't look very pretty.

Current

image

Proposal

A couple of things I explored:

  • Moving glyph area to the left of line numbers. This helps create separation between between the scm indicators and a clear area for adding comments.
  • Adding button for adding and view comments. This follows a similar pattern on GitHub for reviewing changes.
  • Hiding diff chunk areas. This reduces the visual noise and I think most people will be commenting on changed lines in the PR as opposed to lines that are not related
  • Adding color to the + and - changes to look visually pleasing

Kapture 2021-02-04 at 16 36 03

Unsure who should I assign to so assigning to @rebornix and @alexdima for now
@miguelsolorio miguelsolorio added ux User experience issues diff-editor Diff editor issues comments Comments Provider/Widget/Panel issues labels Feb 5, 2021
@miguelsolorio
Copy link
Contributor Author

Some additional feedback from the team:

  • We might still need the indicator for comment areas, particularly when viewing files with changes in review mode where the diffs are not present
    image
  • We could also surface the "add comment" button on line hover to increase discoverability

@alexdima alexdima added the feature-request Request for new features or functionality label Feb 5, 2021
@microsoft microsoft deleted a comment Feb 5, 2021
@alexdima alexdima modified the milestones: Backlog Candidates, On Deck Feb 5, 2021
@alexdima alexdima removed the diff-editor Diff editor issues label Oct 15, 2021
@alexdima alexdima removed their assignment Oct 15, 2021
@rebornix rebornix modified the milestones: On Deck, Backlog Nov 5, 2021
@miguelsolorio miguelsolorio mentioned this issue Apr 5, 2022
Open
52 tasks
@miguelsolorio miguelsolorio mentioned this issue May 9, 2022
Closed
@alexr00 alexr00 modified the milestones: Backlog, July 2022 Jun 14, 2022
@alexr00 alexr00 assigned alexr00 and unassigned rebornix Jun 14, 2022
@alexr00
Copy link
Member

alexr00 commented Jun 14, 2022

I agree that we will likely still need to show the range, but let's improve the leave-a-comment and there's-a-comment decorations.

@alexr00 alexr00 mentioned this issue Jul 21, 2022
Closed
@alexr00 alexr00 modified the milestones: July 2022, August 2022 Jul 22, 2022
@kieferrm kieferrm mentioned this issue Aug 7, 2022
Closed
@alexr00
Copy link
Member

alexr00 commented Aug 8, 2022 •
edited
Loading

Here's what I'm thinking:

Recording 2022-08-08 at 14 31 12 (1)

  • It uses the comment codicon for lines with a comment thread.
  • Keeps the existing + for add
  • Makes them both look a little more clickable but making them more square and giving them rounded corners.

@misolori and @daviddossett what do you think? I didn't add any color for now and I kept everything to the right of the line numbers still.

@alexr00 alexr00 mentioned this issue Aug 8, 2022
Merged
alexr00 added a commit that referenced this issue Aug 8, 2022
@alexr00
Proposal for updating commenting area for diffs
e6531e4 
Fixes #115808
@miguelsolorio
Copy link
Contributor Author

@alexr00 this is looking great, love that we can have codicon support in the comments! I know you mentioned you hadn't added any color, is that something that you'd be open for us to add? I did a quick mockup that shows an alternate styling:

CleanShot 2022-08-08 at 08 57 41@2x

@alexr00
Copy link
Member

alexr00 commented Aug 8, 2022 •
edited
Loading

I didn't add color because I thought the blue was a bit too strong, but I really like the color in this mock up. I'll add it and merge the change!

I also see that you're using the codicon +. I'll switch to that too.

@vscodenpa vscodenpa added the unreleased Patch has not yet been released in VS Code Insiders label Aug 10, 2022
joyceerhl pushed a commit that referenced this issue Aug 10, 2022
@alexr00 @joyceerhl
Proposal for updating commenting area for diffs (#157508)
f117a2e 
* Proposal for updating commenting area for diffs
Fixes #115808

* Polish changes
GulajavaMinistudio pushed a commit to GulajavaMinistudio/vscode-1 that referenced this issue Aug 10, 2022
@alexr00
Proposal for updating commenting area for diffs (microsoft#157508)
b290ec3 
* Proposal for updating commenting area for diffs
Fixes microsoft#115808

* Polish changes
@vscodenpa vscodenpa added insiders-released Patch has been released in VS Code Insiders and removed unreleased Patch has not yet been released in VS Code Insiders labels Aug 11, 2022
@alexr00
Copy link
Member

alexr00 commented Aug 15, 2022

Also updated the multiline add:

image

@alexr00 alexr00 mentioned this issue Aug 15, 2022
Merged
alexr00 added a commit that referenced this issue Aug 15, 2022
@alexr00
Update multiline comment add UX
800dacc 
Part of #115808
pull bot pushed a commit to zhengkaiyuan1993/vscode that referenced this issue Aug 15, 2022
@alexr00
Update multiline comment add UX (microsoft#158163)
c3b497a 
Part of microsoft#115808
@alexr00 alexr00 mentioned this issue Aug 22, 2022
Closed
3 tasks
@ghost ghost mentioned this issue Sep 6, 2022
Closed
@github-actions github-actions bot locked and limited conversation to collaborators Sep 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@alexr00 alexr00

Labels
comments Comments Provider/Widget/Panel issues feature-request Request for new features or functionality insiders-released Patch has been released in VS Code Insiders on-testplan ux User experience issues
Projects
None yet
Milestone
August 2022
Development

Successfully merging a pull request may close this issue.

Proposal for updating commenting area for diffs microsoft/vscode
6 participants
@rebornix @alexdima @miguelsolorio @alexr00 @vscodenpa and others