Skip to content

Resolve "Discussion "expand"/"collapse" button is only clickable on one side"

What does this MR do?

Changes the gutter area between the diff file tree and the diff file list to be a margin instead of a padding.

Problem

Since the discussion expand/collapse button is offset "outside" the box for the file list, anything that has a higher layering order in that area will block mouse events from getting to the part that is being overlapped.

The file tree has an explicit z-index of 202, and the file list has an explicit z-index of 201.

Since the discussion expand/collapse buttons are in the file list, they can never overlay the file tree box.

Complications

The reason for this z-index layering is the drag bar to resize the tree. In the current layout, the drag bar appears on top of the discussion expand/collapse button. This MR assumes that this should continue to be the case. If it were to be allowed to be below the expand/collapse buttons, we could remove the explicit z-indexes and apply a few other small styles. Generally-speaking, I believe that "natural layering" is preferable, but I assume there was a good reason for reversing the layering, so I've left it.

Solution

The solution I chose here was to change the padding-right of the tree to be a margin-right, which isn't part of the mouse-event blocking box model, and set the translateX of the drag bar to be 10px.

Worth noting here: the previous value was -6px, and the new value is 10px. This checks out logically since the previous translation was moving it left into its own padding ($gl-padding: 16px) and the new translation is moving it right into its margin (.mr-3: $spacer * 2, where $spacer is 8px).

Outcome

The discussion expand/collapse button is almost fully clickable. The exception to this is where the draggable resize bar overlaps it, so it's about 90% clickable.

Visual Comparison

Before After
See the issue (63905) for a demo of previous behavior simplescreenrecorder-2019-08-12_19.20.58

Does this MR meet the acceptance criteria?

Conformity

Performance and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • ~~Label as security and @ mention @gitlab-com/gl-security/appsec~~
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team

Closes #63905 (closed)

Edited by 🤖 GitLab Bot 🤖

Merge request reports

Loading