GitLab Next

What does this MR do?

This MR primarily adds the ability to comment on image diffs. This includes merge requests and commit diffs. When you click on an image in the image diff, you can create discussions. Each time you click on the image (on a different area/coordinate), you will create a new discussion.

In addition, we also did the following on the frontend:

• Consolidated the number of places where we initialize gl.ImageFile()
• Converted btn-comment into a mixin, so that it can be reused for image diffs
• Fixed an edge case on gl.ImageFile whereby occasionally the images wouldn't trigger the 2-up view because the image was loaded before gl.ImageFile was invoked

I don't see the collapse icon SVG anymore. How do I get my local system setup?

1. Clone https://gitlab.com/gitlab-org/gitlab-svgs locally
2. Checkout branch add-collapse-icon (at least until gitlab-svgs!5 (merged) gets merged)
3. Run yarn install (first time only)
4. Run yarn run svg
5. Replace dist/icons.svg (gitlab-svg repo) with app/assets/images/icon.svg (this repo)

Are there points in the code the reviewer needs to double check?

Make sure everything looks 💯

Why was this MR needed?

Deliverable

Screenshots (if relevant)

Architecture

Diffs	MR Discussion tab

commenting on gif in MR diff tab	commenting on gif in discussion tab

Replaced image in MR diff tab	Replaced image in discussion tab

commenting on commit diff	commit diff discussion in MR

Does this MR meet the acceptance criteria?

• Changelog entry added, if necessary

• Documentation created/updated (Will create in another MR)

• Tests added for this feature/bug

• Review

  • Has been reviewed by UX (@dimitrieh)

  • Has been reviewed by Frontend

  • Has been reviewed by Backend

  • Has been reviewed by Database

• Conform by the merge request performance guides

• Conform by the style guides

• Squashed related commits together

What are the relevant issue numbers?

closes #35873 (closed), #38559 (closed)

---

Remaining Todos:

Remaining items:

• Disable commenting when not logged in (@brycepj)
• jagged borders (@brycepj)
• update badge counters on avatar when new comment in an existing discussion is added (@ClemMakesApps)
• update badge counters on avatar when comment is edited (@ClemMakesApps)
• update badge counters on image when comment is deleted (@ClemMakesApps)
• toggle collapse of image diff discussion (@brycepj)
• fix jump to unresolved discussion (@ClemMakesApps)
• enable image diff on multiple image views (@ClemMakesApps)
• highlight when linked (fix multiple highlights) (@ClemMakesApps)
• Add image diff to discussion tab (@ClemMakesApps)
• Fix z-index of collapsed discussion badge overlap with jagged border (@ClemMakesApps)
• Unable to comment on images if the image loads collapsed by default
• Commenting doesn't work properly when it is replaced view but the 2-up, swipe, onion skin modes don't kick in (related to https://gitlab.com/gitlab-org/gitlab-ce/issues/38559)
• Ensure Flash message is shown on comment error (@brycepj)
• Make image badges and avatar badges round circles (https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/14061#note_41928050) (6cbd2ff3 @brycepj)
• Update blue dot in image diff discussion (on discussion tab) to use the diff comment image (https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/14061#note_41928050)
• Add blue commenting dot to author of image diff discussion (on discussion tab) (https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/14061#note_41928050) (@ClemMakesApps)
• no blue halo around pointers in images, see https://gitlab.com/gitlab-org/gitlab-ce/issues/35873#note_41934515 (9d680897 @brycepj)
• After an image diff discussion is resolved (and you refresh the MR in the discussion tab), and expand the resolved discussion, the comments do not appear (@ClemMakesApps)
• Center align collapse/badge number buttons in image diff discussions (diff tab)
• Increase collapse toggle button size in image diff discussions to match badge number button size (diff tab)
• Check unbindEvents method to make sure binded events are removed
• Add jagged border between last discussion and new discussion comment form
• Get polling working with image diff (@brycepj)
• Review scss
• Comment form bug https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/14061#note_42080525MakesApps
• Remove duplicate from /javascripts/diff_notes/icons/collapse_icon.svg (created ~"technical debt" )
• Glitch every time we click changes tab (https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/14061#note_42116310) (@ClemMakesApps)
• Focus halo is still present (https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/14061#note_42116310) (@ClemMakesApps)
• When collapsing a discussion after having focussed on the editor to create a reply when it was open.. the editor stays in view.. either in small mode or big mode (https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/14061#note_42116310)
• Initialize replaced image view modes on discussion-tab (@ClemMakesApps)
• Center align the diff collapse icon
• Adding new discussion on diff tab doesn't automatically add avatar badge anymore

Tests

Karma Tests

• badge_helper
• comment_indicator_helper
• dom_helper
• utils_helper
• image_badge
• image_diff
• init_discussion_tab
• replaced_image_diff
• view_types
• image_utility

Rspec Tests

• Clicking on image creates image diff discussion
• Resolved diff discussions load collapsed
• Adding a comment to an existing diff discussion shows avatar badge
• Polling a comment shows avatar badge
• Image diff view modes show discussion badges (on discussion tab)
• Image diff view modes show discussion badges (on diffs tab)
• Image diff displays on discussion tab
• Image diff displays on diffs tab
• Image diff displays on commit diff

Pushed to next iteration

1. avatar tooltips when hovering over badge on the image diff
2. miniature version of the chat, when collapsing a discussion
3. clicking on image badges should scroll to first note in the discussion (both discussion and changes tab, in a sensible way which tries to keep the image in context with the first comment in the discussion)
4. add shadows to badge and cursor so that it's easier to visualize the depth
5. create image diff discussions on the older image to another iteration
6. clicking on the edge of the image in swipe view mode causes the comment indicator to get clipped (https://tppr.me/oAbGP)
7. 2-up view isn't always placed side by side. We should change it so that it is. I think it is displayed on top of each other when the image width is over a certain size

Known UX debt (acceptable to move forward know that these exist, will create individual issues for each of them before this gets merged and approved by @dimitrieh)

1. when you delete the first note of a discussion, the avatar badge does not get populated for the new first note (until you refresh the page) - https://gitlab.com/gitlab-org/gitlab-ce/issues/38974

2. when you edit the first note of a discussion, the avatar badge does not get displayed after the editing is saved (until you refresh the page) - https://gitlab.com/gitlab-org/gitlab-ce/issues/38975

3. If you do scenario 1 and then you switch to the other tab (discussion if you were on diff tab, diff tab if you were on discussion) and assuming those two tabs are already loaded, the avatar badge will not get populated (until you refresh the page) - https://gitlab.com/gitlab-org/gitlab-ce/issues/38978

4. If you do scenario 2 and then you switch to the other tab (discussion if you were on diff tab, diff tab if you were on discussion) and assuming those two tabs are already loaded, the avatar badge will not get populated (until you refresh the page) - https://gitlab.com/gitlab-org/gitlab-ce/issues/38978

5. Toggling nav bar and sidebar does not update the image badge positions to match their positions even though the image size changes - https://gitlab.com/gitlab-org/gitlab-ce/issues/38976