Skip to content

Scroll exactly to the top of a discussion on the MR Overview tab

Thomas Randolph requested to merge defect/jump-to-next-overscroll into master

What does this MR do?

For #227061 (closed)

This MR is intended to have one effect: remove the buffer (called context in code) above a discussion that was just scrolled to.

This is accomplished with these two changes:

  • In the discussion navigator component mixin, if the discussion is in the Overview tab, don't use the ...WithContext function, which intentionally adds a 10% buffer above the scroll location
  • In the common utilities file, update the contentTop computation to ignore the diff file header if it's not in the Changes tab.
    • The file header is already within the element being scrolled to, so there's no reason to subtract the header height from the scroll location.

One additional refactor is included in this MR:

  • The complexity of the computation for the various heights has increased slightly - from a cyclomatic complexity of 9 to a cyclomatic complexity of 10.
    • Instead of doubling down on extremely high cyclomatic complexity, the contentTop function is now a list of "getters" for the various heights
    • This results in an outer (main function) complexity of 1, and an inner complexity (in the most complex getter) of 4, which is still high, but a much more acceptable level of complexity.

Screenshots (strongly suggested)

Before After
before-desktop-compressed after-desktop-compressed

Does this MR meet the acceptance criteria?

Conformity

Availability 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
Edited by Thomas Randolph

Merge request reports