Commenting on a changed image in a merge request diff
On Sep 11, there was a discussion between Victor, Felipe, Dimitrie, Jacob and Clement. Considering how we want to make MR notes/comments/discussions in Vue in the future, frontend wants to write the image diff commenting component in Vue. Since MR notes/comments/discussions is not currently in Vue, adding the image diff component to the discussion tab may prove to be an area of potential concern. As a result, the team has concluded that we are willing to add a link on the discussion tab for image comments that will open up the changes tab and scroll to the image discussion (when there are discussions). When we get closer to the point where we can determine the risk factor of this part of the scope, we will further discuss and examine what content we want to display on the link so that the user still has some contextual information about the image discussion.
Description
- Allow commenting on a changed image on an MR diff. Only for mr diff. Not for commit diffs.
- There should be some symmetry/consistency with https://gitlab.com/gitlab-org/gitlab-ee/issues/1769.
- We will limit the scope to just commenting on a particular coordinate of the image. (Not selecting an area.) Just a particular point in the image.
- Commenting on the entire image is out of scope.
Previous description
This place should then be indicated with a number, that shows the number of comments there.
Zeplin.io, Phabricator and invisionapp.com already do this.
Update (Oct 19 2016)
The original scope of this issue was just 'diffs'. After several discussions, we have decided that there is more value with starting this work from the perspective of 'issue' comment threads.
Design
I'll be working from the following Figma document (this is also inspectable for developers)
Diff tab
Creating a new discussion thread
Hover:
On click:
Example of new thread on an image with multiple existing threads:
Open threads on images
Single thread:
With multiple threads:
Collapsed threads on images (may be done in an iteration if need be)
Collapsing works similarly to code diffs:
1 collapsed thread:
1 collapsed thread, if there is only 1 thread:
Multiple collapsed threads:
Coordinate linking to threads
Each avatar will get a coordinate number indicator:
Coordinate statuses
When hovering over the pointer within the image, it works similarly to the avatars in code diffs (only the pointer itself is first summarising that particular thread):
Discussion separation with jagged borders
In order to separate discussions on the same image, we are using a jagged border. In the following codepen you can find an example for implementation: https://codepen.io/strages/pen/BdZEGL
Image diff modes active
Discussion tab
In the discussion thread, each thread gets its own image replica. This is similar to code diffs. There are no coordinate number indicators necessary, as there is always only 1 thread. Therefore the avatar can be used.
Image diff modes
Creating a new discussion thread (may be done in an iteration if need be)
On hover:
On click:
After click, new thread will be at the bottom of the discussion tab:
Cursor svg's:
Calculating coordinates
- When you first click on an image to add a pin to start a discussion, GitLab knows the relative position of the image, along the x and y axes, independently.
- For example, suppose the relative position is 50% along the length of the x axis, and 25% along the y axis.
- Suppose that that the length of the x axis is 500 units and the y axis is 100 units. Then the position is (x=250, y=25).
- Later on, when the image has changed (because there has been a new push to the same file name of the image), the relative positions remain the same. I.e. the pin's x position is always 50% along the new length of the x axis, and the the pin's y position is always 25% along the new length of the y axis. So for example, suppose the newest image (of the same file name) has x axis length 100 units and the y axis length 100 units. Then the new latest position is (x=50, y=25)
- Note that with the above scheme, we do not care if the aspect ratio of the image is changing.
- Note also that pins will never be "outside" the image.
edge case
Side-by-side 2-up image comment creation:
- you can click anywhere on either image to start a marker, and in the same relative location on the other image, a marker would appear there. This works for both 2-up and swipe if you squint your eyes and think about it. For onion, it’s not as well defined because the images are overlapping (@ClemMakesApps will follow up on this)
Image diffs containing new and old images had links to the image commit files. In other words, if an image diff had a replaced image, the old image and the new image when clicked, redirects the user to the image commit file.
- Functionality will be lost and replaced by an extra button, like:
