Update line-customTooltips.html (Re issue #4038 )

[el-ee]

Fix for #4038

Add window scroll position to tooltip position calculation so they show up in the intended place instead of off-screen or otherwise detached from the graph.

[simonbrunel]

@el-ee should we also fix pie-customTooltips.html and dataPoints-customTooltips.html?

I think that also fixes #3995 and ideally we should remove the use of getBoundingClientRect(), move the tooltip under the canvas parent container and use offsetTop and offsetLeft instead. That can be done while reworking examples for another PR.

[el-ee]

@simonbrunel Are these changes what you are saying would be better? (Aside: I did not realize they would show up automatically as part of this existing PR .. but I guess they do. Sorry if that is annoying. I am not an expert at contributing to other people's projects on here.)

Anyway, I can put either change -- the first one I suggested or this alternate one -- into all three of the files; I hadn't looked at the pie or dataPoints examples before. Let me know.

[simonbrunel]

@el-ee your last change looks good, thanks. I think using offsetTop and offsetLeft is the correct way to position a tooltip because it doesn't require to deal with scrolling position. @etimberg what's your thought?

I have no objection if you want to also update the two other samples :)

[etimberg]

This looks good to me 😄 If it's possible to update the other 2 samples as well, I'd say we should do it to be consistent

[el-ee]

cool yea I can add it to the other 2 samples later today.

[etimberg]

@el-ee if you can, please also rebase this against the sample file rename changes that got submitted tonight.

[el-ee]

ok, i will have to figure out how to do that tomorrow.

[etimberg]

@el-ee the Git docs on rebase are here: https://git-scm.com/docs/git-rebase

In general what you will need to do is:

1. git checkout master
2. git fetch
3. git pull
4. git checkout <your branch>
5. git rebase master

During the rebase process you'll hit a conflict for the samples/line/line-customTooltips.html file which will need to be resolved by making changes in https://github.com/chartjs/Chart.js/blob/master/samples/tooltips/custom-line.html

[simonbrunel]

@el-ee apparently something went wrong, did you force push your changes to GitHub after the rebase: git push --force origin master? (where origin is your fork)

[el-ee]

@simonbrunel I did not. I can do that now.

That codeclimate error looks like it failed to run the fixme engine at all though, rather than that the check actually failed? The details page says: "We had trouble running the fixme engine." ... "docker: Error response from daemon: shim error: context deadline exceeded." And then down below the error message it confusingly has a second box that says passed. IDK.

[simonbrunel]

I think codeclimate fails because of the error in the commit history. You can try to force push your local master: only your 4 commits should appear in that PR.

[el-ee]

ok. it seems possible that I have fixed the weird commit history and rebased properly this time ... fingers crossed.

[simonbrunel]

I think the two newly fixed examples also need <div id="chartjs-tooltip"> to be moved under the canvas container: <div id="canvas-holder1"> (I didn't test, so maybe I'm wrong)?

[el-ee]

Ah, good point. In these files, they show up visually in the same place, regardless of whether they're inside the container in the DOM hierarchy. So, I left them where they were to err on the side of changing fewer lines. But yes, upon reflection, if you added a position:relative to the canvas container div, then it would get weird, and then they would need to be inside it to get the right location. I can move them inside. That is way more resilient for people copying and pasting this code into other files, which is certainly what got me into this mess in the first place! Hah.

[etimberg]

Looks good to me. 😄