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

Riken-Shah commented 3 years ago

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

Go to README.md#Troubleshooting

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:

1.

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;
}

rtfpessoa commented 3 years ago

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.

Riken-Shah commented 3 years ago

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>

rtfpessoa commented 3 years ago

@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.

ve3 commented 3 years ago

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.)

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.

rtfpessoa commented 3 years ago

@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.

ve3 commented 3 years ago

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 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.

rtfpessoa commented 3 years ago

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.

ve3 commented 3 years ago

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.

Shimada666 commented 2 years ago

modify css

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

have a try

rtfpessoa commented 2 years ago

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

fatihpense commented 2 years ago

@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.

rtfpessoa commented 2 years ago

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.

aldifirmansyah commented 2 years ago

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 ?

Riken-Shah commented 2 years ago

@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.

ve3 commented 2 years ago

@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

nghiepdev commented 2 years ago

Same issue, but to fix just using

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

SherCong commented 1 year ago

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

have a try ,that fixed my problem

Githamza commented 6 months ago

Any updates, that was fixed since ?

ve3 commented 6 months ago

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?

Evntual commented 4 months ago

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):

After (only one scrollbar, works perfectly):

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);
}

rtfpessoa / diff2html