Skip to content

Fix SCSS interpolation on MR sticky sidebar height and margin-bottom

Scott de Jonge requested to merge mr-sticky-sidebar-height into master

What does this MR do and why?

The height and margin-bottom properties for the moved MR sticky sidebar weren't being computed from SCSS variables to CSS values.

The sidebar was previously using a negative margin-bottom to compensate for $content-wrapper-padding (100px) at the bottom of the viewport.

The height property allows the sidebar to be scrolled independently of the viewport at smaller heights (or when sidebar content extends past the viewport height).

SCSS variables need to be interpolated within calc() blocks for the values to be computed

Before:

@media (min-width: 992px) {
  .right-sidebar.right-sidebar-expanded .issuable-sidebar.is-merge-request .issuable-context-form {
    height: calc(calc(100vh - calc(var(--header-height, 48px) + calc(var(--system-header-height) + var(--performance-bar-height)) + var(--top-bar-height)) - var(--system-footer-height)) - 76px - var(--mr-review-bar-height) - $content-wrapper-padding);
    margin-bottom: calc((var(--header-height, 48px) + $issue-sticky-header-height) * -1);
  }
}

After:

@media (min-width: 992px) {
  .right-sidebar.right-sidebar-expanded .issuable-sidebar.is-merge-request .issuable-context-form {
    height: calc(calc(100vh - calc(var(--header-height, 48px) + calc(var(--system-header-height) + var(--performance-bar-height)) + var(--top-bar-height)) - var(--system-footer-height)) - 72px - var(--mr-review-bar-height));
    margin-bottom: calc((100px * -1) + var(--mr-review-bar-height));
  }
}

Changes

  • Update local $issue-sticky-header-height variable to use global $mr-sticky-header-height variable
  • Remove $content-wrapper-padding from height calculation
  • Update margin-bottom to compensate for bottom padding

Screenshots or screen recordings

Before After
CleanShot_2023-06-06_at_12.19.09 CleanShot_2023-06-06_at_14.31.30

How to set up and validate locally

  1. Enable Feature.enable(:moved_mr_sidebar)
  2. View MR page e.g. http://127.0.0.1:3000/flightjs/Flight/-/merge_requests/4
  3. Scroll to bottom of page
    1. Right sidebar should not scroll up
  4. Decrease height so sidebar contents are longer than viewport height
    1. Scroll inside sidebar contents without scrolling viewport

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Scott de Jonge

Merge request reports