diff2html icon indicating copy to clipboard operation
diff2html copied to clipboard

Line Number Row is overflowing when it's wrapped inside a fixed height div.

Open Riken-Shah opened this issue 3 years ago • 20 comments

Step -1: Before filling an issue check out troubleshooting section

Step 0: Describe your environment

  • OS: Mac OS Mojave
  • diff2html version: 3.4.3
  • Using diff2html directly or using diff2html-ui helper: diff2html directly
  • Extra flags: _

Step 1: Describe the problem:

When wrapping the HTML coming from diff2html.html in fixed-sized div line numbers column seems to be broken.

The below screenshot best describes the issue, Screenshot 2021-04-11 at 2 01 14 PM

Steps to reproduce:

const diffHtml = diff2html.html(diffString, {
                drawFileList: true,
                matching: "lines",
                outputFormat: 'side-by-side',
});
$content.html(diffHtml);

diff example:

===================================================================
--- backend-markdown
+++ frontend-markdown
@@ -3,11 +3,9 @@
 <a href="https://github.com/jgm/commonmark.js">commonmark.js</a>, the<br>
 JavaScript reference implementation.</p>
 <ol>
 <li>item one</li>
-<li>
-<p>item two</p>
-<ul>
+<li>item two<ul>
 <li>sublist</li>
 <li>sublist</li>
 </ul>
 </li>
@@ -17,11 +15,9 @@
 <a href="https://github.com/jgm/commonmark.js">commonmark.js</a>, the<br>
 JavaScript reference implementation.</p>
 <ol>
 <li>item one</li>
-<li>
-<p>item two</p>
-<ul>
+<li>item two<ul>
 <li>sublist</li>
 <li>sublist</li>
 </ul>
 </li>

Observed Results:

  • What happened? This could be a description, log output, etc. Already explained above.

Expected Results:

  • What did you expect to happen? Line numbers do not overflow.

Relevant Code:

Overridding the line number position fixes the issue,

.d2h-code-side-linenumber{
    position: relative;
}

Riken-Shah avatar Apr 11 '21 08:04 Riken-Shah

Hi @Riken-Shah,

I cannot replicate this behavior, even with ridiculously small widths the lines never break. Can you provide a complete HTML example where it breaks? You can check https://github.com/rtfpessoa/diff2html/issues/380#issuecomment-822433106 as a starting point for the example.

rtfpessoa avatar Apr 25 '21 10:04 rtfpessoa

Hey @rtfpessoa,

I might have missed mentioning that it breaks inside a fixed-sized div where overflow = scroll.

Here is the HTML code, where you can quickly reproduce the behavior (Try to scroll),

<!DOCTYPE html>
<html>
<head>
  <title>Report</title>
<!-- CSS -->
<link rel="stylesheet" type="text/css" href="https://cdn.jsdelivr.net/npm/diff2html/bundles/css/diff2html.min.css" />
<!-- Javascripts -->
<script type="text/javascript" src="https://cdn.jsdelivr.net/npm/diff2html/bundles/js/diff2html.min.js"></script>
</head>
<body>
<script>
  document.addEventListener('DOMContentLoaded', () => {
    const diffString = `===================================================================
--- backend-markdown
+++ frontend-markdown
@@ -3,11 +3,9 @@
<a href="https://github.com/jgm/commonmark.js">commonmark.js</a>, the<br>
JavaScript reference implementation.</p>
<ol>
<li>item one</li>
-<li>
-<p>item two</p>
-<ul>
+<li>item two<ul>
<li>sublist</li>
<li>sublist</li>
</ul>
</li>
@@ -17,11 +15,9 @@
<a href="https://github.com/jgm/commonmark.js">commonmark.js</a>, the<br>
JavaScript reference implementation.</p>
<ol>
<li>item one</li>
-<li>
-<p>item two</p>
-<ul>
+<li>item two<ul>
<li>sublist</li>
<li>sublist</li>
</ul>
</li>`;

        const configuration = { outputFormat: 'line-by-line', drawFileList: false, matching: 'lines' };
        const html = Diff2Html.html(diffString, configuration);
        document.getElementById("code-block").innerHTML = html;

  })
</script>
<div style="margin: 5%;">
  <pre id="code-block" style="height: 500px; overflow: scroll;"></pre>
</div>
</body>

Riken-Shah avatar Apr 25 '21 12:04 Riken-Shah

@Riken-Shah After replacing the pre tags with a div I can see the issue. But it cannot be fixed by changing the position to relative since that makes the horizontal scroll hide the line numbers.

Unfortunatly I do not have a solution atm for that scenario. Let me know if you have ideas.

rtfpessoa avatar May 07 '21 21:05 rtfpessoa

I think it is the same as this issue https://github.com/rtfpessoa/diff2html/issues/185

Add position:relative to .d2h-diff-table class will work and line number still show the same across browsers. The CSS did not wrap element that have position:absolute that's why it will be always break when it is not in the normal element. (in the element that have limited height, overflow scroll will break position:absolute if there is no position:relative wrap on it.)

Read more about this at -> https://css-tricks.com/absolute-positioning-inside-relative-positioning/ and -> https://dzone.com/articles/css-position-relative-vs-position-absolute and -> https://developer.mozilla.org/en-US/docs/Web/CSS/position

absolute The element is removed from the normal document flow, and no space is created for the element in the page layout. It is positioned relative to its closest positioned ancestor, if any; otherwise, it is placed relative to the initial containing block. Its final position is determined by the values of top, right, bottom, and left

relative The element is positioned according to the normal flow of the document, and then offset relative to itself based on the values of top, right, bottom, and left. The offset does not affect the position of any other elements; thus, the space given for the element in the page layout is the same as if position were static.

This mean... td with position:absolute will be working fine on static page. Scroll page down, the td will be move up as its position. But if td with position:absolute is in the div with height: 300px; overflow: auto;, when scroll it does not scroll the page but scroll the div but td position is still stick to the page -> then it's broken. Wrap position:absolute with position:relative on its parent element fix this problem and did not break the layout.

ve3 avatar Jul 21 '21 15:07 ve3

@ve3 like I said before. If I do that change I always get it broken on horizontal scroll.

You really need to provide an example where it is not broken. I cannot replicate the fix.

rtfpessoa avatar Jul 23 '21 20:07 rtfpessoa

I don't see any horizontal scroll broken. https://codepen.io/ve3/pen/LYyOdNQ

Checked on Firefox, Chrome.

How does it broken? side by side or some special configuration?

Update: Oh! I see that when H scroll to the right, the line number disappear. Is that?

In this case... Reference to GitHub Screenshot-2021-07-24-at-10-14-53-rtfpessoa-diff2html-Pretty-diff-to-html-javascript-library-diff2h.png from this page https://github.com/rtfpessoa/diff2html/commit/6572b68bf26fa264c1e6b6babca777ced2982c77

I try to locate the element with class file js-file js-details-container js-targetable-element and then add custom style

width: 500px;
overflow: auto;
height: 400px;

to see how is it on GitHub and the result as seen in screenshot. They are just ignore horizontal scroll for line numbers.

In this case, I see that H scroll on line numbers and limited height inside wrapped element can't go together without additional JS to render special element. So, for @Riken-Shah or anyone who have this problem including me... I have to add special CSS (postition relative) and choose to ignore H scroll for line numbers to keep it display properly in limited height element.

ve3 avatar Jul 24 '21 02:07 ve3

The example you provided is broken. The line numbers are supposed to always be visible. When you scroll then disappear.

That only happens due to the change.

rtfpessoa avatar Jul 24 '21 22:07 rtfpessoa

I was updated the example -> https://codepen.io/ve3/pen/LYyOdNQ

The code below can help fix horizontal scroll with position relative.

// @link https://stackoverflow.com/a/8676457/128761 Fix horizontal scroll.
var leftOffset = parseInt($(".d2h-code-side-linenumber").css('left')); //Grab the left position left first
$('.d2h-file-side-diff').scroll(function(){
    $('.d2h-code-side-linenumber').css({
        'left': $(this).scrollLeft() + leftOffset //Use it later
    });
});

It is not very good result but in case who want to keep both working, this maybe help.

ve3 avatar Jul 25 '21 08:07 ve3

modify css

.d2h-code-side-linenumber {
  position: relative;
  display: table-cell;
}
.d2h-code-side-line {
  padding: 0 0.5em;
}

have a try

Shimada666 avatar Aug 31 '21 13:08 Shimada666

@Shimada666 not sure what you were expecting but it does not work.

rtfpessoa avatar Sep 01 '21 15:09 rtfpessoa

@rtfpessoa, Maybe it is a different but related bug. Inserted HTML creates double scrollbars if a parent div is scroll=auto & fixed height. @Shimada666 's CSS fixed this problem for me.

fatihpense avatar Sep 11 '21 23:09 fatihpense

Yes, the problem we are trying to solve here is to have a limited height, but the HTML and CSS are not ready for this. It would need a re-write to make it work without breaking the current functionality.

rtfpessoa avatar Sep 18 '21 20:09 rtfpessoa

I encounter the same issue. Are there any chance that this will be fixed? Or did you fix this issue on your project @Riken-Shah ?

aldifirmansyah avatar Sep 24 '21 09:09 aldifirmansyah

@aldifirmansyah, No I also tried but I can't seem to fix it. I am just using this CSS rule, to make things a little better

.d2h-code-side-linenumber{
    position: relative;
}

As @rtfpessoa said, using this will create another issue,

But it cannot be fixed by changing the position to relative since that makes the horizontal scroll hide the line numbers.

Riken-Shah avatar Sep 27 '21 12:09 Riken-Shah

@aldifirmansyah, No I also tried but I can't seem to fix it. I am just using this CSS rule, to make things a little better

.d2h-code-side-linenumber{
    position: relative;
}

As @rtfpessoa said, using this will create another issue,

But it cannot be fixed by changing the position to relative since that makes the horizontal scroll hide the line numbers.

Maybe my sample might help https://codepen.io/ve3/pen/LYyOdNQ

ve3 avatar Sep 27 '21 13:09 ve3

Same issue, but to fix just using

.d2h-diff-tbody tr {
  position: relative;
}

nghiepdev avatar Dec 27 '21 08:12 nghiepdev

.d2h-wrapper {
    transform: translateZ(0);
 }
.d2h-diff-table {
    position: relative;
 }
.d2h-code-side-linenumber {
    position: fixed;
}

have a try ,that fixed my problem

SherCong avatar Nov 01 '22 03:11 SherCong

Any updates, that was fixed since ?

Githamza avatar Feb 02 '24 09:02 Githamza

Due to the fixes maybe conflict with stick line numbers. Stick line numbers is default option for this repo. So, I think what if you add some option to do not stick line numbers and use one of our CSS to fix the problem?

ve3 avatar Feb 02 '24 09:02 ve3

Just came across this issue while trying to show the html inside a scrollable Dialog component from Material UI. Adding transform: translateZ(0); to .d2h-wrapper fixed my issue.

Before (see the two scroll bars): image

After (only one scrollbar, works perfectly): image

Copied the original .d2h-wrapper content from diff2html/bundles/css/diff2html.min.css and included the modified version in my index.css:

.d2h-wrapper {
  text-align: left;
  transform: translateZ(0);
}

Evntual avatar Apr 06 '24 15:04 Evntual