anseki / leader-line

Draw a leader line in your web page.
http://anseki.github.io/leader-line/
MIT License
3.03k stars 423 forks source link

Line start and and end anchor not accurate on windows #343

Closed Camsteack closed 2 years ago

Camsteack commented 2 years ago

Hi,

First of all thanks a lot for this library it's awesome ! But I seem to have a problem only on windows, regardless of the browser (Chrome, Edge, Firefox) where the start and end anchor are not positioned properly. Everything works as expected on OSX regardless of the browser as well:

OSX: Screenshot 2022-07-06 at 10 30 06

WINDOWS: Screenshot 2022-07-06 at 10 30 24

The lines are created using the start and end DOM elements:

const end = document.getElementById(`f${feature.id}`);
const start = document.getElementById(`d${dep.id}`);
if (end && start) {
  const line = new LeaderLine(start, end, { hide: true });
  line.show('draw');
}

Am I doing something wrong ? Many thanks ! Camille

anseki commented 2 years ago

Hi @Camsteack, thank you for the comment. I think that the reason is that in the time to create the line because the gap looks like width of scroll bar. In other words, the elements were moved after the lines were drawn properly. For test, wrap your code with this:

addEventListener('load', () => {
  // Your code here
});
Camsteack commented 2 years ago

Hi @anseki ! Thanks a lot for the quick reply ! So the lines are drawn on hover so the elements are already rendered and not moving when the line is created, so it should not change right ? I am also moving the lines to a wrapper div instead of the body when they are created so it moves properly when I scroll the table. But I tried to remove it to make sure it wasn't that causing the issue and I get the same behaviour

anseki commented 2 years ago

Do you mean that you removed the code that moves the lines to a <div> element? To reproduce the issue, could you show me an example by using e.g. https://jsfiddle.net/ ?

Camsteack commented 2 years ago

Yes, the full code when I move it to the wrapper is:

const end = document.getElementById(`f${feature.id}`);
            const start = document.getElementById(`d${dep.id}`);
            if (end && start) {
              const line = new LeaderLine(start, end, { hide: true });
              moveLinesToWrapper();
              line.show('draw');
              return line;
            }

And the moveLinesToWrapper methods is:

const moveLinesToWrapper = () => {
    const wrapper = document.getElementById('wrapper');
    wrapper.style.transform = 'none';
    const wrapperRect = wrapper.getBoundingClientRect();
    const renderedLines = document.querySelectorAll('.leader-line');
    wrapper.style.transform =
      'translate(-' + (wrapperRect.left + pageXOffset) + 'px, -' + (wrapperRect.top + pageYOffset) + 'px)';
    renderedLines.forEach((line) => document.getElementById('wrapper').appendChild(line));
  };

This allow me to scroll the table with the lines moving correctly not staying fixed. But I removed the moveLinesToWrapper call to make sure this was not causing the issue and I get the same behavior.

I am going to try to reproduce it in fiddle, not sure if I ll be able to !

anseki commented 2 years ago

Ok, I will wait for the example.

Camsteack commented 2 years ago

Hi @anseki, I have tried to replicate on a codesandbox with kind of the same setup here https://codesandbox.io/s/react-leaderline-forked-h8mf5q

But unfortunately it seems to be ok, could you think of anything that could create that difference between Windows and OSX ? Thanks !

anseki commented 2 years ago

People say that Safari is worst web browser now. That has many bugs and doesn't support many new web specification and has many problems about security. Anyway, try to remove the moveLinesToWrapper();. If it works fine, the moveLinesToWrapper has bug. And also, honestly, I feel that your code is strange... For example, 'translate(-' + (wrapperRect.left + pageXOffset) + 'px... might be 'translate(--123px..., etc.

Camsteack commented 2 years ago

Yeah I already tried without the wrapper and the behavior is the same. What s weird is that it s not related to browser as all browsers on windows are not working and all browser on osx are :/

anseki commented 2 years ago

For test, try this:

setTimeout(() => {
  const line = new LeaderLine(document.getElementById(`d${dep.id}`), document.getElementById(`f${feature.id}`));
}, 3000);
Camsteack commented 2 years ago

Still the same. One last chance but sometimes it seems that leaderline thinks the element has moved to the line below when it hasn't, here is an example:

Screenshot 2022-07-06 at 15 21 30

I am hover over the chip 102 at the end of the line but leaderLine seems to think it has wrapped to the line below so the line starts from there. Again only happening on windows browser :/

Camsteack commented 2 years ago

Also just to note, I got the logic for the moveToWrapper from a jsfiddle you provided https://jsfiddle.net/2sfxvy8k/ :)

anseki commented 2 years ago

If you write the test code correctly, the result is never same because setTimeout should delay creating a line. Therefore, the test was not done correctly. Maybe, your code has simple bug. I think that the reason is that in the time. BTW, that jsfiddle is not my code. You seem to have mistaken @christian-iron60's jsfiddle code that I edited it. And your moveLinesToWrapper and position in the jsfiddle are not same. As I said, the moveLinesToWrapper has bugs.

Camsteack commented 2 years ago

Sorry I wasn't clear. When I said still the same I meant the problematic behaviour is still the same. The delay is applied but the start and end positions are not correct. Yes sorry my mistake I thought it was from you. I ll try to find out what the issue is. Thanks !

anseki commented 2 years ago

Ok. :smile: Anyway, the moveLinesToWrapper is not the reason of this issue because you said that you already removed lines that call moveLinesToWrapper. Also, nobody can find a bug in your code without reading your code. I think that the reason is that in the time but I am uncertain because I don't know your code and you didn't reproduce the issue.

Camsteack commented 2 years ago

So I think I found the issue ! You were right saying that it's related to the scrollbar being there. Because one of the parent container has an overflow: hidden the scroll bar doesn't appear, but for some reason leaderline expects the scroll bar to be there so the start / end are moved. It might be that my html/css structure is not the best way to achieve the scroll I am looking for or maybe it's a bug. I ll try to reproduce it in a code sandbox now that I know what the problem is !

Camsteack commented 2 years ago

Ok, it is finally working properly by adding overflow: hidden to the body ! There was no scroll there to start with so I am not sure why that makes a difference ?

anseki commented 2 years ago

I see. The reason is the time to create the line. The elements were moved after the lines were drawn properly. https://github.com/anseki/leader-line/issues/343#issuecomment-1176078210 Also, screen size may affect the showing scroll bars.

Camsteack commented 2 years ago

I don't think that's the case as the elements are not moving when the line is drawn as it's only on hover when everything is already rendered. I ll see if I can reproduce the HTML hierarchy on codesandbox to reproduce :)

anseki commented 2 years ago

I see. Of course I don't know why you think that the elements are not moved after rendering. I thought that your app loads external resources such as image files that changes the layout. Anyway, if you reproduce the issue, it may be solved.

anseki commented 2 years ago

No reply came, then this abandoned issue is closed.