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-index
es 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 itsmargin
(.mr-3: $spacer * 2
, where$spacer
is8px
).
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
-
Changelog entry for user-facing changes, or community contribution. Check the link for other scenarios. -
Documentation created/updated or follow-up review issue created -
Code review guidelines -
Merge request performance guidelines -
Style guides -
Database guides -
Separation of EE specific content
Performance and Testing
-
Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process. -
Tested in all supported browsers
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)