search

wri / gfw-mapbuilder

Template for the GFW Map Builder that is available through ArcGIS Online, as a stand-alone web application, & a library to build custom Forest Atlas web applications
https://my.gfw-mapbuilder.org/v1.latest/
MIT License
33 stars 10 forks source link

Tooltips are incorrect in report charts #466

Closed richardbarad closed 5 years ago

richardbarad commented 5 years ago

In the report, the tooltips are not placed correctly in the biomass chart on the right hand side.

To replicate

  1. Hover over the 2017 bar in the carbon emissions graph in report .
  2. The data visible is for 2018 as shown in screenshot below.

image

The tooltips are also not showing up in the report for the GLAD alerts chart when you hover over a point as shown below. Seems like it must related to the sizing since the tooltips work correctly in the layer panel. Any ideas?

image

kaylakremer commented 5 years ago

@richardbarad , I'm not able to replicate the issue you're having with the tooltips. They appear to hover fine for me, even when I adjust the window size and make the chart small. (I am using Chrome. Perhaps it behaves differently in different browsers?)

I went ahead and made a new build with the latest code that's been merged in our base branch for the report redesign. Could you test here and let me know if you have the same issue? http://alpha.blueraster.io/gfw-mapbuilder/466-tooltips/

richardbarad commented 5 years ago

@KaylaKremer. I am having this issue across all browsers! I just tested in firefox as well as Edge (the new report does not load in edge). Creating a new ticket for that and marking it as a high priority than this. https://github.com/wri/gfw-mapbuilder/issues/480. Dropping high priority here.

I am attaching a video so that you can see what I am seeing on my side with the tooltips:

For the biomass chart, the tooltips work sometimes but on the right hand side of the chart they show the wrong year. For GLAD, they do not work at all. They are working for the other charts so maybe it has something to do with resizing those two chart types.

Let me know if you are still not able to replicate!

tooltips.zip

kaylakremer commented 5 years ago

@richardbarad ,

Thanks for the information as well as the screen recording! I see what you mean by it not working for you.

I just tried it out again on my end with the same build. I'm still not sure about the Biomass Chart, but at least for the GLAD chart, it seems like the tooltip only appears if you hover exactly over the dot. Which is a bit tricky since it's so small. I've attached my screen recording demonstrating what I mean. http://alpha.blueraster.io.s3.amazonaws.com/gfw-mapbuilder/466-tooltip-video/Screen%20Recording%202019-08-05%20at%2011.01.41%20AM.zip

Perhaps it's simply just a mouse/touchpad precision issue, which is why we are getting different results? The only other solution I can come up with is doing something similar from this example I found on Vega's site: https://vega.github.io/vega/examples/bar-chart/

If you scroll down, you'll see in the marks, there is an update property for both the x and y with a signal for the tooltip. It looks like they added an offset to this, so maybe we need to do the same. I've attached a screenshot for what I'm talking about:

Screen Shot 2019-08-05 at 11 24 55 AM

In any case, I plan to ask @SwampGuzzler about this today since I'm not quite sure how to tackle this ticket.

richardbarad commented 5 years ago

Hi @KaylaKremer.

Thank you for these details and for researching this! I was aware of this issue with the tooltip appearing only when you hover directly over the point so it is great to know about the offset option. I'll test that out in coming weeks and see if that helps.

However, regarding the report it looks like the tooltips do not show up even when I hover directly over the points on my side. I am attaching a video showing this behavior. It looked like the tooltips worked correctly for you in both the biomass & GLAD charts so maybe it is some sort of PC/mac issue? I am using Chrome Version 75.0.3770.142. I also noticed that your tooltips have a slightly different formatting in the the browser and are on grey background.

Let's see what @SwampGuzzler and we can also chat about this when we meet tomorrow.

tooltips_report.zip

richardbarad commented 5 years ago

@KaylaKremer & @SwampGuzzler,

Let's make a ticket in the VEGA github and see if they have a suggestion for this!

kaylakremer commented 5 years ago

@richardbarad ,

I happened to come across this issue in Vega's github: https://github.com/vega/vega/issues/1201

EDIT: Also found this: https://github.com/vega/vega-tooltip/issues/120 Looks like the tooltips were fixed in Vega 4

I can still make an issue on their github, but they may just tell us to upgrade to at least version 4 as mentioned in that thread.

richardbarad commented 5 years ago

@kaylakremer. I am not sure if this is the same issue.

It seemed like our issue was specific to resizing the widgets in the report and only happened when using a PC? Were you able to replicate on your side using a PC?

kaylakremer commented 5 years ago

@richardbarad ,

I haven't tested it out myself with a PC. I'll ask @csphang about it. I'll make the issue on Vega's github if we encounter the same issue on our end.

kaylakremer commented 5 years ago

@richardbarad ,

I just tested the tooltips out on a Windows PC and I got the same results as you did. I will go ahead and make an issue on Vega's Github about this.

The only slight difference with my results was that for the bar chart, I got the hover to show up on the empty space in between the purple bars. I noticed it was also slightly off for the horizontal bar charts as well.

kaylakremer commented 5 years ago

@richardbarad , I posted in Vega's official Slack since our issue I think is a specific use case.

I took a screenshot of what I posted. I will let you know once I receive any response. Or if you would like to post on their Slack too, it's vega-js.slack.com, #vega-tooltip channel.

Screen Shot 2019-08-13 at 1 52 33 PM

kaylakremer commented 5 years ago

@richardbarad Response from someone at Vega:

Screen Shot 2019-08-14 at 9 51 25 AM

@csphang Since I think we've decided that upgrading Vega to the latest version isn't a good option, can we thoroughly test this behavior on all browsers for both PC/Mac this week? If we get consistent results, then I will open up the ticket on the Vega repo. They'll need an example to look at, so should I provide them a URL to one of our builds? (Not sure how much I should be sharing externally from this project!)

csphang commented 5 years ago

@KaylaKremer Confirmed this is happening for me on PC across browsers. Testing specs below.

Test URL: http://alpha.blueraster.io/gfw-mapbuilder/new-report-consolidated-builds/default/ Chrome Version 76.0.3809.100 Firefox Version 68.0.1 Microsoft Edge 44.18362.1.0

kaylakremer commented 5 years ago

@richardbarad @csphang

I opened up an issue on Vega's Github here: https://github.com/vega/vega/issues/1925

richardbarad commented 5 years ago

@KaylaKremer. Thanks! I saw they provided a response and just followed up. It looks they tested in the analysis panel instead of in the report so the tolltips worked correctly. Sent a message to clarify.

kaylakremer commented 5 years ago

@richardbarad I just checked their message. Thanks for replying to them!

kaylakremer commented 5 years ago

@richardbarad ,

It looks like the Roy guy we've been communicating with at Vega has responded with some possible solutions for the tooltip. I went ahead and implemented his first suggestion of using svg instead of canvas.

I don't have access to a Windows PC at the moment, but if you could test with this new build and let me know if the tooltip hover works for you, that would be great!

http://alpha.blueraster.io/gfw-mapbuilder/466-tooltips/

csphang commented 5 years ago

Tooltips work so much better on that build! @richardbarad hopefully you have the same experience - the issue was resolved for me on the new build @KaylaKremer posted above.

richardbarad commented 5 years ago

@csphang and @KaylaKremer . Yes, they seem to be working much better now! I am noticing that the charts seems to load a bit slower (i.e: the time when we see the blank white space while the charts renders are the VEGA response is returned seems to be longer).

Are you seeing this as well? If yes, do you think we should consider one of the other recommendations Roy shared?

Richard

kaylakremer commented 5 years ago

Hi @richardbarad , I'm not noticing much of a difference on my end. Roy's other suggestion was just to try removing the max-width or the rem units, which are both CSS solutions. That wouldn't affect how fast the VEGA response returns.

richardbarad commented 5 years ago

@KaylaKremer. If we went with that solution would the resize still work?

richardbarad commented 5 years ago

@KaylaKremer. I just tested in Edge and noticed some unusual behavior with tooltips in edge. They seem to sometimes persist in location and will not move when you hover to a new a bar. How hard would it be to set up a build with the other option to compare performance?

edge_tooltips.zip

kaylakremer commented 5 years ago

@richardbarad If we remove the max-widths, then the charts will no longer be responsive in how we want. Remember, we are setting a max-width on all the chart's containers. Without this, they will not render how we want them to.

As for using rem units versus pixels, I used rem values for many of the report CSS styles. I would have to recalculate the pixel equivalent for every rem unit I used. And then, we would possibly run into the issue of things no longer appearing correctly in different browsers/screen sizes since pixels are a fixed unit of measure whereas rem is scalable.

@SwampGuzzler Could you weigh in on this? Do you think it's worth converting over our CSS rem unit styles?

csphang commented 5 years ago

@richardbarad We tested on Edge on multiple PC's at the office using both a trackpad and a mouse, and were unable to reproduce the issue. We might have to do a screenshare with you to troubleshoot? Let us know if this minor or a showstopper for the release. Potentially we can move this to the backlog while we move onto the other items.

richardbarad commented 5 years ago

@csphang. I think this is minor and not a reason to hold up a release. I'll move this to accepted column, I think we should keep an eye on how some of the other reports behave once we merge and switch to svg instead of canvas..