Skip to content
This repository has been archived by the owner on Jul 13, 2024. It is now read-only.

Fix incorrect font height in labels and tags when percent width is used #155

Merged
merged 1 commit into from
Apr 18, 2017

Conversation

geksiong
Copy link
Contributor

When canvas is contained in a div with percent width, _getFontHeight will get an incorrect font height. Using 'display: inline-block' in the dummy div gets the correct result.

@nicoespeon
Copy link
Owner

Hi @geksiong, thanks a lot for this!

Do you have a snippet/fiddle to reproduce the issue? I didn't managed to get incorrect result doing this:

<div style="width: 30%;">
  <canvas id="gitGraph"></canvas>
</div>

Font height doesn't seem to change − I'm using Chrome v57.

@geksiong
Copy link
Contributor Author

I forgot to mention I floated the div too. Without the float the graph looks fine.

@nicoespeon
Copy link
Owner

I didn't manage to reproduce the issue with

<div style="width: 30%; float: left;">
  <canvas id="gitGraph"></canvas>
</div>

Is there anything special on the <body> − since it's used to determine the font height?
Could you provide a snippet to help me reproduce the bug?

Thanks a lot!

@geksiong
Copy link
Contributor Author

geksiong commented Apr 14, 2017

hi @nicoespeon here's a code snippet demonstrating the bug, modified from examples/index.html. Basically I'm trying to do a 2-column layout.

<!DOCTYPE HTML>
<html lang="en">
 
  <head>
    <meta charset="UTF-8">
    <title>GitGraph.js example page</title>
    <link rel="stylesheet" type="text/css" href="../src/gitgraph.css" />
    <style>
      body {
        margin: 0;
        padding: 0;
      }
      #graph {
        width: 60%;
        float: left;
      }
      #detail {
        width: 40%;
        float: right;
      }
    </style>
  </head>
 
  <body>
    <div id="graph">
      <canvas id="gitGraph"></canvas>
    </div>
    <div id="detail">Lorem ipsum dolor sit amet, consectetur adipisicing elit. Sint, ducimus, qui fuga corporis veritatis doloribus iure nulla optio dolores maiores dolorum ullam alias cum libero obcaecati cupiditate sit illo aperiam possimus voluptatum similique neque explicabo quibusdam aspernatur dolorem. Quod, corrupti magni explicabo nam sequi nesciunt accusamus aliquam dolore! Cumque, quam fugiat ab veritatis. Quia, maxime quas perferendis cupiditate explicabo at atque iusto accusamus. Nesciunt veniam quidem nemo doloribus! Dolore, cupiditate, adipisci, voluptate quam nihil ipsa placeat dolor possimus minus quas nostrum eaque in dicta autem eligendi rerum facilis nesciunt sunt doloremque suscipit enim iure vitae eius voluptates tempora tenetur hic.</div>
    <script src="../src/gitgraph.js"></script>
    <script src="index.js"></script>
  </body>

</html>

I was incorrect about the percent widths being the cause of the bug. It is the 2 floated divs together that causes the bug. A single floated div by itself is ok. And if you remove the width attributes the problem is still there.

@nicoespeon
Copy link
Owner

Hi @geksiong!

I managed to reproduce it with:

<div style="float: left;">
  <canvas id="gitGraph"></canvas>
</div>

And I can confirm your fix is working. Thanks a lot and sorry for bothering you to reproduce the issue, I needed to dig into the root cause to check for regression later 👍

@nicoespeon nicoespeon merged commit a7175b5 into nicoespeon:develop Apr 18, 2017
@nicoespeon
Copy link
Owner

FYI, this fix is part of release v1.10.1.

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

Successfully merging this pull request may close these issues.

2 participants