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 chart tooltips broken on Firefox #7822

Closed
3 tasks done
CoryChaplin opened this issue Jul 4, 2019 · 12 comments · Fixed by #7929
Closed
3 tasks done

Line chart tooltips broken on Firefox #7822

CoryChaplin opened this issue Jul 4, 2019 · 12 comments · Fixed by #7929
Labels
browser:firefox Related to the Firefox browser !deprecated-label:bug Deprecated label - Use #bug instead good first issue Good first issues for new contributors .pinned Draws attention viz:charts:line Related to the Line chart

Comments

@CoryChaplin
Copy link
Contributor

A clear and concise description of what the bug is.

Expected results

Hover point on the line, see a tooltip.

Actual results

No visible tooltip on Firefox (but it works on Chrome).

Screenshots

Capture d’écran 2019-07-04 à 17 18 16

How to reproduce the bug

  1. Go to Explore view or a Dashboard with a line chart
  2. Hover a point on the line
  3. See no tooltip

Environment

(please complete the following information):

  • superset version: 0.33.0.rc1
  • python version: 3.6
  • node.js version: 12.5.0
  • npm version: 6.1.0

Checklist

Make sure these boxes are checked before submitting your issue - thank you!

  • I have checked the superset logs for python stacktraces and included it here as text if there are any.
  • I have reproduced the issue with at least the latest released version of superset.
  • I have checked the issue tracker for the same issue and I haven't found one similar.
@issue-label-bot issue-label-bot bot added the !deprecated-label:bug Deprecated label - Use #bug instead label Jul 4, 2019
@issue-label-bot
Copy link

Issue-Label Bot is automatically applying the label #bug to this issue, with a confidence of 0.99. Please mark this comment with 👍 or 👎 to give our bot feedback!

Links: app homepage, dashboard and code for this bot.

@schoel-bis
Copy link
Contributor

I am seeing the same here. This is due to #7102, more precisely the style for the body element in superset/assets/stylesheets/less/index.less.

The position: absolute setting on the body element causes Firefox to calculate the height (and consequently the clientHeight) of the html element as 0 as absolutely positioned elements have no influence on their parent's height. But NVD3 uses document.documentElement.clientHeight (documentElement is the html element) in the formula for calculating the position of the tooltip. Since it is always 0 on Firefox the calculation produces a grossly false result and places the tooltip off-screen.

My understanding of the change from the pull request above is that is meant to ensure that the body will always cover the entire window. A similar effect should be achievable with something like the following while avoiding the problem described above:

html {
  height: 100%;
}

body {
  min-height: 100%;
  display: flex;
  flex-direction: column;
}

@etr2460
Copy link
Member

etr2460 commented Jul 26, 2019

@schoel-bis Are you sure this is the issue here? I've been investigating this same issue and am pretty sure the root cause is here: https://github.com/apache-superset/superset-ui-plugins/blob/master/packages/superset-ui-legacy-preset-chart-nvd3/src/NVD3Vis.js#L1081

We're removing all tooltips on the page whenever we rerender a chart, which is problematic especially on dashboards where multiple charts could have tooltips in the DOM. If your fix solves the issue here, than awesome, but I think there's another deeper problem that needs to be solved with the nvd3 chart plugin

@schoel-bis
Copy link
Contributor

Well, frankly I wouldn't bet on this being the one and only problem with tooltips or NVD3 on Firefox or anywhere. Anyway, I wasn't seeing any tooltips whatsoever in Firefox and with that patch applied I have them back.

@CoryChaplin
Copy link
Contributor Author

@xtinec maybe you can confirm this as a side effect of your commit ?

schoel-bis added a commit to schoel-bis/incubator-superset that referenced this issue Aug 7, 2019
This bug was introduced by apache#7102

Using `position:absolute` on body gives `document.documentElement` a height of 0 which propagates to `clientHeight` on Firefox. This leads to a wrong calculation of the tooltip position in NVD3.

The solution proposed here is to use `min-height: 100vh` instead of the current technique for stretching the body element to the full window height, thus keeping body and html together.
mistercrunch pushed a commit that referenced this issue Aug 9, 2019
This bug was introduced by #7102

Using `position:absolute` on body gives `document.documentElement` a height of 0 which propagates to `clientHeight` on Firefox. This leads to a wrong calculation of the tooltip position in NVD3.

The solution proposed here is to use `min-height: 100vh` instead of the current technique for stretching the body element to the full window height, thus keeping body and html together.
graceguo-supercat pushed a commit to airbnb/superset-fork that referenced this issue Aug 30, 2019
graceguo-supercat pushed a commit to airbnb/superset-fork that referenced this issue Aug 30, 2019
[Revert] Line chart tooltips broken on Firefox apache#7822
etr2460 pushed a commit to etr2460/incubator-superset that referenced this issue Aug 30, 2019
@graceguo-supercat
Copy link

graceguo-supercat commented Aug 30, 2019

@CoryChaplin This PR caused an issue: the standalone mode chart didn't show anymore. please see #8147 for details. We have reverted this PR from master branch.

@schoel-bis
Copy link
Contributor

So, if the fix has been reverted, this needs to be re-opened, I suppose. I can't find how to do that, though.

Although I do not currently have the time to investigate any further, I'd like to add a few observations:

@michellethomas michellethomas reopened this Sep 3, 2019
@stale
Copy link

stale bot commented Nov 2, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. For admin, please label this issue .pinned to prevent stale bot from closing the issue.

@stale stale bot added the inactive Inactive for >= 30 days label Nov 2, 2019
@stale stale bot closed this as completed Nov 9, 2019
@amicis31
Copy link

amicis31 commented Feb 6, 2020

Could you open a more general issue or assign .pinned to this?

@magor
Copy link

magor commented Mar 26, 2020

Hi, we are experiencing the same issue with latest stable release, I understand the fix was reverted?

@saward
Copy link

saward commented Jul 19, 2020

I'm having an issue like this, where hovering mouse over charts with firefox does not display any extra information.

@mistercrunch mistercrunch reopened this Jul 23, 2020
@stale stale bot removed the inactive Inactive for >= 30 days label Jul 23, 2020
@mistercrunch mistercrunch added .pinned Draws attention browser:firefox Related to the Firefox browser preset-io labels Jul 23, 2020
@junlincc junlincc added the chart label Nov 25, 2020
@junlincc junlincc added viz:charts:line Related to the Line chart and removed chart labels Jan 2, 2021
@junlincc junlincc added the good first issue Good first issues for new contributors label Mar 31, 2021
@betodealmeida
Copy link
Member

This seems to be working on Firefox now, I couldn't repro.

Screen Shot 2021-04-21 at 1 40 37 PM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
browser:firefox Related to the Firefox browser !deprecated-label:bug Deprecated label - Use #bug instead good first issue Good first issues for new contributors .pinned Draws attention viz:charts:line Related to the Line chart
Projects
None yet
Development

Successfully merging a pull request may close this issue.