Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
Riken-Shah opened this issue Apr 11, 2021 · 20 comments

Comments

@Riken-Shah
Copy link

Riken-Shah commented Apr 11, 2021

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 Riken-Shah changed the title Line Numbers is overflowing when it's wrapped inside fixed heigh div. Line Number Row is overflowing when it's wrapped inside a fixed height div. Apr 11, 2021
@rtfpessoa
Copy link
Owner

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 #380 (comment) as a starting point for the example.

@Riken-Shah
Copy link
Author

Riken-Shah commented Apr 25, 2021

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
Copy link
Owner

rtfpessoa commented May 7, 2021

@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
Copy link

ve3 commented Jul 21, 2021

I think it is the same as this issue #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.

@rtfpessoa
Copy link
Owner

@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
Copy link

ve3 commented Jul 24, 2021

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 6572b68

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
Copy link
Owner

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
Copy link

ve3 commented Jul 25, 2021

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
Copy link

modify css

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

have a try

@rtfpessoa
Copy link
Owner

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

@fatihpense
Copy link

@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
Copy link
Owner

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
Copy link

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
Copy link
Author

@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
Copy link

ve3 commented Sep 27, 2021

@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
Copy link

Same issue, but to fix just using

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

@SherCong
Copy link

SherCong commented Nov 1, 2022

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

have a try ,that fixed my problem

@Githamza
Copy link

Githamza commented Feb 2, 2024

Any updates, that was fixed since ?

@ve3
Copy link

ve3 commented Feb 2, 2024

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
Copy link

Evntual commented Apr 6, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants