Add chart row limit warning tooltip

[rclement]

Fixes #144

[codecov[bot]]

Codecov Report

Merging #156 (04233e5) into master (c66d8df) will not change coverage. The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master      #156   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            8         8           
  Lines          389       388    -1     
  Branches        73        73           
=========================================
- Hits           389       388    -1     

Files Changed	Coverage Δ
datasette_dashboards/__init__.py	100.00% <ø> (ø)
tests/test_dashboard_chart.py	100.00% <100.00%> (ø)
tests/test_dashboard_chart_embed.py	100.00% <100.00%> (ø)
tests/test_dashboard_view.py	100.00% <100.00%> (ø)

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

[rclement]

@HaveF I have implemented a chart row limit warning tooltip as per your suggestion in discussion #142. Could you review it works as expected on your end please?

In the demo, you can see the query behind the map chart exceeds the default 1000 rows limit: https://chart-row-limit-dd-demo.vercel.app/-/dashboards/job-offers-stats

[rclement]

There seems to be a regression in the dashboard view only: the size of the viewport is extended on the right and an horizontal scroll is available. This might be coming from the tooltip element not playing nice with the global layout...

[HaveF]

@rclement It looks great! I tested it in my local environment and it works perfectly with my Vega Lite case.

[HaveF]

There seems to be a regression in the dashboard view only: the size of the viewport is extended on the right and an horizontal scroll is available. This might be coming from the tooltip element not playing nice with the global layout...

I observed that this only appears within a certain window size range. Does it have something to do with reactive layout?

[rclement]

I observed that this only appears within a certain window size range. Does it have something to do with reactive layout?

The reactive layout is automatically handled by the native CSS Grid system, so there is almost nothing we can leverage there.

However, debugging elements overflowing the viewport, the culprit is coming from tooltip text boxes (visible or hidden) on the right-most charts of the grid:

So I need a better way to display tooltips without screwing-up the layout with overflows...

[rclement]

Ok so after some digging, here is what I've come up with:

• Preventing overflows and dynamically placing tooltips is hard (entire librairies are built for this use-case alone, e.g. Popper.js)
• FYI, Metabase is using Tippy.js, which is overkill for our use case but makes overflow handling a breeze
• The HTML title attribute is a native way to display tooltips on DOM elements

The title attribute is not the sexiest tooltip implementation but is native, requires only HTML (not even CSS not JS) and prevents overflows. So let's use that instead!

@HaveF Can you review the new implementation please? https://chart-row-limit-dd-demo.vercel.app/-/dashboards/job-offers-stats?date_start=2021-01-01

[HaveF]

@rclement It appears to be a reasonable solution without the need for too many dependencies. I also tested it locally with Vega Lite examples, and there were no issues.

However, there is one minor problem - the prompts displayed a bit slowly, taking approximately 2 to 3 seconds. At one point, I thought there were no prompts being displayed at all. If this issue is difficult to address, I believe I can still accept it. It seems a feature of browser, I believe we can accept it.

[rclement]

~However, there is one minor problem - the prompts displayed a bit slowly, taking approximately 2 to 3 seconds. At one point, I thought there were no prompts being displayed at all. If this issue is difficult to address, I believe I can still accept it.~ It seems a feature of browser, I believe we can accept it.

The display latency of the tooltip is the main drawback as I see it, but there is nothing we can do about it as this is a per-browser implementation detail, as you observed.

Let's go with that for now, if there is some tooltip wizard in the audience that want to shime in and propose a dependency-free enhancement we'll check it out!