From f3a3743de8103d8b28bc9bc1bbe9989ca23d2814 Mon Sep 17 00:00:00 2001
From: Stanislav Lashmanov <slashmanov@gitlab.com>
Date: Fri, 2 May 2025 19:19:04 +0400
Subject: [PATCH 1/2] Add content-visibility to text diffs in Rapid Diffs

---
 .../javascripts/rapid_diffs/diff_file.js      |  6 ++++-
 .../components/rapid_diffs/_constants.scss    |  3 +++
 .../components/rapid_diffs/app.scss           |  1 +
 .../rapid_diffs/diff_file_component.scss      | 21 ++++++++++++++---
 .../rapid_diffs/text_file_viewers.scss        | 23 ++++++++++++++-----
 .../rapid_diffs/diff_file_component.html.haml |  2 +-
 .../rapid_diffs/diff_file_component.rb        | 11 +++++++++
 spec/components/rapid_diffs/shared.rb         |  6 +++++
 8 files changed, 62 insertions(+), 11 deletions(-)

diff --git a/app/assets/javascripts/rapid_diffs/diff_file.js b/app/assets/javascripts/rapid_diffs/diff_file.js
index e25d105ac5e8eb..2f57f955644b94 100644
--- a/app/assets/javascripts/rapid_diffs/diff_file.js
+++ b/app/assets/javascripts/rapid_diffs/diff_file.js
@@ -76,7 +76,11 @@ export class DiffFile extends HTMLElement {
   }
 
   selectFile() {
-    this.scrollIntoView();
+    this.scrollIntoView({ block: 'start' });
+    setTimeout(() => {
+      // with content-visibility we might get a layout shift which we have to account for
+      this.scrollIntoView({ block: 'start' });
+    });
     // TODO: add outline for active file
   }
 
diff --git a/app/assets/stylesheets/components/rapid_diffs/_constants.scss b/app/assets/stylesheets/components/rapid_diffs/_constants.scss
index 8394c938b83a48..4e3b7df2d23df9 100644
--- a/app/assets/stylesheets/components/rapid_diffs/_constants.scss
+++ b/app/assets/stylesheets/components/rapid_diffs/_constants.scss
@@ -10,3 +10,6 @@ $code-line-height: calc($code-row-height-target / $code-font-size-target);
 
 $app-header-vertical-padding: $gl-spacing-scale-3;
 $app-vertical-breakpoint: md;
+
+$diff-file-border-width: 1px;
+$diff-file-header-padding-y: $gl-spacing-scale-3;
diff --git a/app/assets/stylesheets/components/rapid_diffs/app.scss b/app/assets/stylesheets/components/rapid_diffs/app.scss
index c5d6004e576036..1e066e7907f84b 100644
--- a/app/assets/stylesheets/components/rapid_diffs/app.scss
+++ b/app/assets/stylesheets/components/rapid_diffs/app.scss
@@ -31,6 +31,7 @@
 
 .rd-app-content {
   flex: 1 0;
+  min-width: 0;
 }
 
 // override .code styles because we can't apply .code directly on the diff file code class
diff --git a/app/assets/stylesheets/components/rapid_diffs/diff_file_component.scss b/app/assets/stylesheets/components/rapid_diffs/diff_file_component.scss
index 604996d2fb85dc..e1ec8c54944b1f 100644
--- a/app/assets/stylesheets/components/rapid_diffs/diff_file_component.scss
+++ b/app/assets/stylesheets/components/rapid_diffs/diff_file_component.scss
@@ -1,10 +1,25 @@
+@use 'constants';
+
 .rd-diff-file-component {
+  display: block;
   scroll-margin-top: var(--rd-app-sticky-top-with-padding);
+  margin-bottom: $gl-padding;
 }
 
 .rd-diff-file {
   --rd-diff-file-border-radius: #{calc($gl-border-radius-base - 1px)};
-  padding-bottom: $gl-padding;
+}
+
+.rd-diff-file[data-content-visibility]:not([data-collapsed]) {
+  $header-padding: constants.$diff-file-header-padding-y * 2;
+  // top/bottom padding + toggle button size, excluding borders
+  $file-header-height: calc($header-padding + $gl-button-small-size);
+  // header top/bottom borders + body border
+  $total-borders: constants.$diff-file-border-width * 3;
+  content-visibility: auto;
+  contain-intrinsic-size:
+    auto 0 auto
+    calc(#{$file-header-height} + #{$total-borders} + (#{constants.$code-row-height-target} * var(--total-lines)));
 }
 
 .rd-diff-file-header {
@@ -15,7 +30,7 @@
   display: flex;
   background-color: var(--gl-background-color-subtle);
   border: 1px solid var(--gl-border-color-default);
-  padding: $gl-spacing-scale-3 $gl-spacing-scale-4;
+  padding: constants.$diff-file-header-padding-y $gl-spacing-scale-4;
   border-radius: var(--rd-diff-file-border-radius) var(--rd-diff-file-border-radius) 0 0;
   word-break: break-word;
   z-index: 1;
@@ -86,7 +101,7 @@
 }
 
 .rd-diff-file-body {
-  border: 1px solid var(--gl-border-color-default);
+  border: constants.$diff-file-border-width solid var(--gl-border-color-default);
   border-top: 0;
   border-radius: 0 0 var(--rd-diff-file-border-radius) var(--rd-diff-file-border-radius);
 
diff --git a/app/assets/stylesheets/components/rapid_diffs/text_file_viewers.scss b/app/assets/stylesheets/components/rapid_diffs/text_file_viewers.scss
index 9a55e8425fafd0..93f5ad2b12c5dc 100644
--- a/app/assets/stylesheets/components/rapid_diffs/text_file_viewers.scss
+++ b/app/assets/stylesheets/components/rapid_diffs/text_file_viewers.scss
@@ -22,21 +22,29 @@
   display: grid;
 }
 
-.rd-hunk-lines:last-child > :first-child {
-  border-bottom-left-radius: var(--rd-diff-file-border-radius);
+.rd-text-view-content tr:last-child {
+  --rd-row-bottom-left-radius: var(--rd-diff-file-border-radius);
+  --rd-row-bottom-right-radius: var(--rd-diff-file-border-radius);
 }
 
-.rd-hunk-lines:last-child > :last-child {
-  border-bottom-right-radius: var(--rd-diff-file-border-radius);
+.rd-text-view-content tr:last-child td:not(:last-child) {
+  --rd-row-bottom-right-radius: 0;
+}
+
+.rd-text-view-content tr:last-child td:not(:first-child) {
+  --rd-row-bottom-left-radius: 0;
 }
 
 .rd-hunk-header {
+  position: relative;
+  // overlap line numbers
+  z-index: 2;
   // this is used when a hunk header doesn't have any text, only expand buttons
   min-height: calc(1em * $code-line-height);
-  border-top: 1px solid var(--diff-expansion-background-color, var(--gl-border-color-default));
-  border-bottom: 1px solid var(--diff-expansion-background-color, var(--gl-border-color-default));
   background-color: var(--code-diff-hunk-header-background-color, $gray-50);
   color: var(--code-diff-hunk-header-color, $gray-400);
+  border-radius: 0 0 var(--rd-row-bottom-right-radius) var(--rd-row-bottom-left-radius);
+  outline: 1px solid var(--diff-expansion-background-color, var(--gl-border-color-default));
 
   &:first-child {
     border-top: 0;
@@ -89,6 +97,7 @@
 
   background-color: var(--code-diff-expand-button-background-color, $gray-100);
   color: var(--code-diff-expand-button-color, var(--gl-text-color-subtle));
+  border-radius: 0 0 var(--rd-row-bottom-right-radius) var(--rd-row-bottom-left-radius);
 
   &:hover,
   &:focus {
@@ -171,10 +180,12 @@
 
 .rd-line-number:first-child {
   box-shadow: -1px 0 var(--rd-line-border-color);
+  border-bottom-left-radius: var(--rd-row-bottom-left-radius);
 }
 
 .rd-line-content:last-child {
   box-shadow: 1px 0 var(--rd-line-border-color);
+  border-bottom-right-radius: var(--rd-row-bottom-right-radius);
 }
 
 .rd-line-link {
diff --git a/app/components/rapid_diffs/diff_file_component.html.haml b/app/components/rapid_diffs/diff_file_component.html.haml
index d770d97314b525..0c3116e37c1a7e 100644
--- a/app/components/rapid_diffs/diff_file_component.html.haml
+++ b/app/components/rapid_diffs/diff_file_component.html.haml
@@ -1,7 +1,7 @@
 -# TODO: add fork suggestion (commits only)
 
 %diff-file.rd-diff-file-component{ id: id, data: {  testid: 'rd-diff-file', **server_data } }
-  .rd-diff-file
+  .rd-diff-file{ data: { content_visibility: total_lines > 0 }, style: ("--total-lines: #{total_lines}" if total_lines > 0) }
     = header || default_header
     -# extra wrapper needed so content-visibility: hidden doesn't require removing border or other styles
     %div{ data: { file_body: '' } }
diff --git a/app/components/rapid_diffs/diff_file_component.rb b/app/components/rapid_diffs/diff_file_component.rb
index f109b5f5c7cdb0..52aa244d7bc056 100644
--- a/app/components/rapid_diffs/diff_file_component.rb
+++ b/app/components/rapid_diffs/diff_file_component.rb
@@ -39,5 +39,16 @@ def viewer_component
     def default_header
       render RapidDiffs::DiffFileHeaderComponent.new(diff_file: @diff_file)
     end
+
+    def total_lines
+      return 0 unless @diff_file.text?
+
+      count = 0
+      @diff_file.viewer_hunks.each do |hunk|
+        count += hunk.header.expand_directions.count if hunk.header
+        count += @parallel_view ? hunk.parallel_lines.count : hunk.lines.count
+      end
+      count
+    end
   end
 end
diff --git a/spec/components/rapid_diffs/shared.rb b/spec/components/rapid_diffs/shared.rb
index 6eab9a352a261e..fccaa26b11ed67 100644
--- a/spec/components/rapid_diffs/shared.rb
+++ b/spec/components/rapid_diffs/shared.rb
@@ -32,6 +32,12 @@ def render_component
     expect(web_component['data-diff-lines-path']).to eq("#{diff_path}/diff_lines")
   end
 
+  it "renders line count" do
+    render_component
+    total_count = web_component.all(:css, 'table tbody tr').count
+    expect(web_component).to have_css("[style~='--total-lines: #{total_count}']")
+  end
+
   context "when is text diff" do
     before do
       allow(diff_file).to receive(:diffable_text?).and_return(true)
-- 
GitLab


From 3fa215952ed312fbd8eb2708edaba6d904ce7722 Mon Sep 17 00:00:00 2001
From: Stanislav Lashmanov <slashmanov@gitlab.com>
Date: Mon, 5 May 2025 16:21:27 +0400
Subject: [PATCH 2/2] Fix z-index, disable on older Chrome, add tests

---
 .../javascripts/rapid_diffs/app/chrome_fix.js   | 11 +++++++++++
 app/assets/javascripts/rapid_diffs/app/index.js |  2 ++
 app/assets/javascripts/rapid_diffs/diff_file.js |  3 +++
 .../rapid_diffs/diff_file_component.scss        |  8 ++++----
 .../rapid_diffs/diff_file_component.html.haml   |  2 +-
 .../rapid_diffs/diff_file_component.rb          | 15 ++++++++++++---
 spec/components/rapid_diffs/shared.rb           | 17 ++++++++++++++++-
 7 files changed, 49 insertions(+), 9 deletions(-)
 create mode 100644 app/assets/javascripts/rapid_diffs/app/chrome_fix.js

diff --git a/app/assets/javascripts/rapid_diffs/app/chrome_fix.js b/app/assets/javascripts/rapid_diffs/app/chrome_fix.js
new file mode 100644
index 00000000000000..0ed51947006f12
--- /dev/null
+++ b/app/assets/javascripts/rapid_diffs/app/chrome_fix.js
@@ -0,0 +1,11 @@
+// content-visibility was fixed in Chrome 138, older versions are way too laggy with is so we just disable the feature
+// https://issues.chromium.org/issues/40066846
+export const disableContentVisibilityOnOlderChrome = () => {
+  if (!/Chrome/.test(navigator.userAgent)) return;
+  const chromeVersion = parseInt(navigator.userAgent.match(/Chrome\/(\d+)/)[1], 10);
+  if (chromeVersion < 138) {
+    document
+      .querySelector('[data-rapid-diffs]')
+      .style.setProperty('--rd-content-visibility-auto', 'visible');
+  }
+};
diff --git a/app/assets/javascripts/rapid_diffs/app/index.js b/app/assets/javascripts/rapid_diffs/app/index.js
index 2dd78d473508e5..d615f806289329 100644
--- a/app/assets/javascripts/rapid_diffs/app/index.js
+++ b/app/assets/javascripts/rapid_diffs/app/index.js
@@ -12,6 +12,7 @@ import { __ } from '~/locale';
 import { fixWebComponentsStreamingOnSafari } from '~/rapid_diffs/app/safari_fix';
 import { DIFF_FILE_MOUNTED } from '~/rapid_diffs/dom_events';
 import { parseBoolean } from '~/lib/utils/common_utils';
+import { disableContentVisibilityOnOlderChrome } from '~/rapid_diffs/app/chrome_fix';
 
 // This facade interface joins together all the bits and pieces of Rapid Diffs: DiffFile, Settings, File browser, etc.
 // It's a unified entrypoint for Rapid Diffs and all external communications should happen through this interface.
@@ -22,6 +23,7 @@ class RapidDiffsFacade {
 
   init() {
     this.#registerCustomElements();
+    disableContentVisibilityOnOlderChrome();
     fixWebComponentsStreamingOnSafari(
       document.querySelector('[data-diffs-list]'),
       this.DiffFileImplementation,
diff --git a/app/assets/javascripts/rapid_diffs/diff_file.js b/app/assets/javascripts/rapid_diffs/diff_file.js
index 2f57f955644b94..2333a1f09d463b 100644
--- a/app/assets/javascripts/rapid_diffs/diff_file.js
+++ b/app/assets/javascripts/rapid_diffs/diff_file.js
@@ -79,6 +79,9 @@ export class DiffFile extends HTMLElement {
     this.scrollIntoView({ block: 'start' });
     setTimeout(() => {
       // with content-visibility we might get a layout shift which we have to account for
+      // 1. first scroll: renders target file and neighbours, they receive proper dimensions
+      // 2. layout updates: target file might jump up or down, depending on the intrinsic size mismatch in neighbours
+      // 3. second scroll: layout is stable, we can now properly scroll the file into the viewport
       this.scrollIntoView({ block: 'start' });
     });
     // TODO: add outline for active file
diff --git a/app/assets/stylesheets/components/rapid_diffs/diff_file_component.scss b/app/assets/stylesheets/components/rapid_diffs/diff_file_component.scss
index e1ec8c54944b1f..eeae9bc7e77baf 100644
--- a/app/assets/stylesheets/components/rapid_diffs/diff_file_component.scss
+++ b/app/assets/stylesheets/components/rapid_diffs/diff_file_component.scss
@@ -10,16 +10,16 @@
   --rd-diff-file-border-radius: #{calc($gl-border-radius-base - 1px)};
 }
 
-.rd-diff-file[data-content-visibility]:not([data-collapsed]) {
+.rd-diff-file[data-virtual]:not([data-collapsed]) {
   $header-padding: constants.$diff-file-header-padding-y * 2;
   // top/bottom padding + toggle button size, excluding borders
   $file-header-height: calc($header-padding + $gl-button-small-size);
   // header top/bottom borders + body border
   $total-borders: constants.$diff-file-border-width * 3;
-  content-visibility: auto;
+  content-visibility: var(--rd-content-visibility-auto, auto);
   contain-intrinsic-size:
     auto 0 auto
-    calc(#{$file-header-height} + #{$total-borders} + (#{constants.$code-row-height-target} * var(--total-lines)));
+    calc(#{$file-header-height} + #{$total-borders} + (#{constants.$code-row-height-target} * var(--total-rows)));
 }
 
 .rd-diff-file-header {
@@ -33,7 +33,7 @@
   padding: constants.$diff-file-header-padding-y $gl-spacing-scale-4;
   border-radius: var(--rd-diff-file-border-radius) var(--rd-diff-file-border-radius) 0 0;
   word-break: break-word;
-  z-index: 1;
+  z-index: 3;
 }
 
 .rd-diff-file-header:has([data-options-toggle] button[aria-expanded='true']) {
diff --git a/app/components/rapid_diffs/diff_file_component.html.haml b/app/components/rapid_diffs/diff_file_component.html.haml
index 0c3116e37c1a7e..2ed79ced44a404 100644
--- a/app/components/rapid_diffs/diff_file_component.html.haml
+++ b/app/components/rapid_diffs/diff_file_component.html.haml
@@ -1,7 +1,7 @@
 -# TODO: add fork suggestion (commits only)
 
 %diff-file.rd-diff-file-component{ id: id, data: {  testid: 'rd-diff-file', **server_data } }
-  .rd-diff-file{ data: { content_visibility: total_lines > 0 }, style: ("--total-lines: #{total_lines}" if total_lines > 0) }
+  .rd-diff-file{ data: { virtual: virtual? }, style: ("--total-rows: #{total_rows}" if virtual?) }
     = header || default_header
     -# extra wrapper needed so content-visibility: hidden doesn't require removing border or other styles
     %div{ data: { file_body: '' } }
diff --git a/app/components/rapid_diffs/diff_file_component.rb b/app/components/rapid_diffs/diff_file_component.rb
index 52aa244d7bc056..e354e77fdd066e 100644
--- a/app/components/rapid_diffs/diff_file_component.rb
+++ b/app/components/rapid_diffs/diff_file_component.rb
@@ -25,7 +25,7 @@ def server_data
     end
 
     def viewer_component
-      return Viewers::NoPreviewComponent if @diff_file.collapsed? || !@diff_file.modified_file?
+      return Viewers::NoPreviewComponent if empty_diff?
 
       if @diff_file.diffable_text?
         return Viewers::Text::ParallelViewComponent if @parallel_view
@@ -36,12 +36,16 @@ def viewer_component
       Viewers::NoPreviewComponent
     end
 
+    def empty_diff?
+      @diff_file.collapsed? || !@diff_file.modified_file?
+    end
+
     def default_header
       render RapidDiffs::DiffFileHeaderComponent.new(diff_file: @diff_file)
     end
 
-    def total_lines
-      return 0 unless @diff_file.text?
+    def total_rows
+      return 0 unless !empty_diff? && @diff_file.diffable_text?
 
       count = 0
       @diff_file.viewer_hunks.each do |hunk|
@@ -50,5 +54,10 @@ def total_lines
       end
       count
     end
+
+    # enables virtual rendering through content-visibility: auto, significantly boosts client performance
+    def virtual?
+      total_rows > 0
+    end
   end
 end
diff --git a/spec/components/rapid_diffs/shared.rb b/spec/components/rapid_diffs/shared.rb
index fccaa26b11ed67..13e488df8ddd7e 100644
--- a/spec/components/rapid_diffs/shared.rb
+++ b/spec/components/rapid_diffs/shared.rb
@@ -35,7 +35,12 @@ def render_component
   it "renders line count" do
     render_component
     total_count = web_component.all(:css, 'table tbody tr').count
-    expect(web_component).to have_css("[style~='--total-lines: #{total_count}']")
+    expect(web_component).to have_css("[style~='--total-rows: #{total_count}']")
+  end
+
+  it "enables virtual rendering" do
+    render_component
+    expect(web_component).to have_css("[data-virtual]")
   end
 
   context "when is text diff" do
@@ -74,6 +79,11 @@ def render_component
       render_component
       expect(web_component['data-viewer']).to eq('no_preview')
     end
+
+    it "disables virtual rendering" do
+      render_component
+      expect(web_component).not_to have_css("[data-virtual]")
+    end
   end
 
   context "when file is collapsed" do
@@ -85,5 +95,10 @@ def render_component
       render_component
       expect(web_component['data-viewer']).to eq('no_preview')
     end
+
+    it "disables virtual rendering" do
+      render_component
+      expect(web_component).not_to have_css("[data-virtual]")
+    end
   end
 end
-- 
GitLab