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

Customizing tooltips #102

Open
joakimkejser opened this issue Dec 6, 2016 · 19 comments
Open

Customizing tooltips #102

joakimkejser opened this issue Dec 6, 2016 · 19 comments

Comments

@joakimkejser
Copy link
Contributor

Hi,

First of all, thanks for a great library!

I have some questions regarding tooltips:

Sometimes, I would like to use displayOptions and a data property to customize a visualization, but I do not want to show the data property in the tooltip - is there a way to avoid that?

Is there an easier way to change the "title" of the tooltips (the one above the legend)? I have managed to do so for a MapMarkerDataLayer, where per default the lat/lon is showed, by using the custom location mode and changing the text property of the location object. However, this seems like a bit of a workaround.

Is it possible to disable the legend in the tooltips?

Finally, is it possible to completely customize the content of the tooltip, or do I need to implement my own with the onEachRecord function?

Regards,
Joakim Kejser

@sfairgrieve
Copy link
Contributor

sfairgrieve commented Dec 7, 2016

Thanks @joakimkejser. Great questions, which should lead to some new features in the future. You should be able to disable the tooltips altogether by setting showLegendTooltips to false in the options you pass in to the DataLayer. You can exclude properties you've called out in displayOptions by setting excludeFromLegend to true under the options associated with a given display option.

In terms of the legend title, I'm assuming you're referring specifically to the location text value that's displayed? The only way to set this now is through the workaround you mentioned. I agree that this should be simpler to customize and will add this to the list of updates that need to be made.

It's not easy at the moment to customize the tooltip content, but I'll add that to the list of updates as well.

@joakimkejser
Copy link
Contributor Author

@sfairgrieve, thanks for getting back to me.

excludeFromLegend does indeed exclude the given display option from the legend, but not from the tooltip. This behavior is demonstrated in the "DC Metro Bus Locations" example. I can however also imagine examples where you would want to exclude a displayOptions from the tooltip but include it in the legend (say you dynamically scale the radius/weight with an arbitrary property not meant for display in the tooltip).

_getLegend take the excludeFromLegend option into account, but _bindMouseEvents does not filter legendDetails before passing it on to L.LegendIcon if I have interpreted the code correctly. Maybe there should be an excludeFromTooltip option?

I am specifically referring to the location text value that is displayed, yes. In relation to that, the L.Graph layer disregards the location.text value of the to and from location retrieved and instead displays the fromField and toField value. This works great in the airport example but leaves no room for customization. I believe text: fromValue + ' - ' + toValue should instead be text: fromLocation.text + ' - ' + toLocation.text here, which would yield the same result in the airport example.

Another option I would like to see is the ability to also hide the legend box (the colored/styled little icon) in the tooltips. I find them very useful in some cases, but when using a MapMarkerDataLayer unstyled or with very few color options, I would like to exclude them.

Once again, thank you for your work on this project - it is immensely useful.

Let me know if there is anything I can do to help.

Regards,
Joakim

@sfairgrieve
Copy link
Contributor

You're right. Great points/suggestions. I think these are all things that should be configurable.

You should be able to hide the legend boxes using CSS. Something like:

div.leaflet-div-icon div.legend-box {
   display: none;
}

Let me know if that doesn't work.

I'm definitely open to contributions, so if you feel like implementing any of these suggestions, I'd be happy to accept pull requests. I'll let you know when I start working on these features once I get some free time (hopefully soon).

Scott

@sfairgrieve
Copy link
Contributor

@joakimkejser After reviewing the code, I also forgot that this option existed, but you can pass in an option called dynamicOptions that is a function that takes a record as input and should return an object with a layerOptions property (key/value pairs for styling the legend box) and a legendDetails property for what fields/values to display. So:

var layer = new L.DataLayer(data, {
...
dynamicOptions: function (record) {
    return {
       layerOptions: {
          color: 'red'
       },
       legendDetails: {
          field1: value1
       }
    };
}
...
});

The _getDynamicOptions function of L.DataLayer is the default implementation of this. Can you give this a try and see if it works? I think this would essentially solve some of your issues, although it's more complex than the options you suggested. I'm looking into adding those in as well.

@joakimkejser
Copy link
Contributor Author

@sfairgrieve Yes, that does indeed work with a small change:

dynamicOptions: function(record) {
    opt = layer._getDynamicOptions(record);
    return {
        layerOptions: opt.layerOptions,
        legendDetails: {
            "whatever": {
                 "name": "Name",
                 "value": record.value
            }
        }
    }
}

I called the original function to avoid having to define all my layerOptions again, but this is also a nice way to allow complete control over those. The legendDetails need a "name" and "value" for it to work.

Is there a way to call _getDynamicOptions without referencing my layer instance?

Thanks! This solves my problem for now. I will see if I can help with some of the other options when I get time.

@sfairgrieve
Copy link
Contributor

Great! Yeah, I just noticed that dynamicOptions is not being passed the DataLayer instance as the context (I'll fix that). Once I fix that, you should be able to call this._getDynamicOptions(record). Saw your pull request as well and will try to accept that tonight. Your help and input is definitely appreciated!

@joakimkejser
Copy link
Contributor Author

joakimkejser commented Dec 14, 2016

@sfairgrieve the pull request is just a minor change, so no hurries. Asesome, the context will be very useful in dynamicOptions.

I have stumbled upon another issue related to tooltips. If showLegendTooltips is set to false, setHighlight and unsetHighlight cannot be used to create a hover effect, as these are bound in the _bindMouseEvents which is only called if showLegendTooltips is true.

A solution could be to move the check on showLegendTooltips to the _bindMouseEvents, as to only exclude tooltip related stuff and leave in the setHighlight and unsetHighlight bit.

A workaround for now is to leave the showLegendTooltips to true, and do:

showLegendTooltips: true,
tooltipOptions: {
  iconSize: new L.Point(0,0),
  iconAnchor: new L.Point(-5000,5000)
}

It is a bit of a hack, but it gets the tooltip out of the way.

@sfairgrieve
Copy link
Contributor

@joakimkejser I pushed some updates to master and 1.0dev that address some of your suggestions. Can you give these updates a try when you get a chance? I did some testing, but I'm hoping you can validate things to make sure I didn't miss anything. I may also break out your suggestions into separate issues, so I can track them better.

@joakimkejser
Copy link
Contributor Author

@sfairgrieve Thanks for working on this. When I load in 1.0dev, everything works fine, but master gives me an error. I think I've tracked it down to some issue with L.ArcedPolyline where the implementation of initialize is different between master and 1.0dev. To be fair, I previously used the distribution used in the examples: http://humangeo.github.io/leaflet-dvf/dist/leaflet-dvf.js, which shares the implementation with the 1.0dev branch. So it is probably not related to your recent changes, and my code might have been faulty all along. I get the following error:

Uncaught TypeError: Cannot read property 'call' of undefined
    at e.initialize (leaflet-dvf.js:7164)
    at new e (leaflet.js:5)
    at e.ARC (leaflet-dvf.js:8045)
    at e._getLocation (leaflet-dvf.js:8082)
    at e._loadRecords (leaflet-dvf.js:5402)
    at e.addData (leaflet-dvf.js:5517)
    at e.initialize (leaflet-dvf.js:5181)
    at new e (leaflet.js:5)
    at HTMLDocument.<anonymous> (report.html:804)
    at i (jquery.min.js:2)

where at HTMLDocument.<anonymous> (report.html:804) is var layer = new L.Graph(data, options);

Maybe you have some inputs on that?

I'll take a look at the specific features you have implemented tomorrow and validate. Just to clarify, from reading the code I understand that you have added/fixed the following:

  • option useLocationText in the L.Graph to switch between using location.text and the value of the fromField/toField as the tooltip 'title'. Furthermore, you have added the option to specify a getLocationText function which gets passed the record and overwrites the tooltip title text (which is a great way to do it by the way).
  • option excludeFromTooltip in the displayOptions of L.DataLayer to exclude a specific displayOption from the tooltip
  • the check on showLegendTooltips have been moved, so now the highlighting functions should work regardless of showLegendTooltips value.

Have I missed anything?

Great idea seperating the issues - sorry for pouring them all into one issue. I can try and create seperate issues and reference the fixes if you'd like?

@sfairgrieve
Copy link
Contributor

Thanks @joakimkejser. That makes sense. master is setup to work with Leaflet 0.7.x and 1.0dev is setup to work with Leaflet 1.0.x. At some point in the near future, I'll switch master to be the 1.0.x compatible version and ween off any updates related to the 0.7.x version. It's tough trying to support both codebases.

The changes you've listed are right. I also updated the code so that the options.dynamicOptions function gets passed this (the DataLayer instance) as the context, so you should be able to call this._getDynamicOptions rather than layer._getDynamicOptions. I'll start adding any of the suggestions I missed as issues tomorrow and begin trying to address them.

@joakimkejser
Copy link
Contributor Author

@sfairgrieve I tested all of the above against the 1.0dev branch, and it works as intended! Reading through our discussions above, the enhancements related to tooltips that are still missing are:

  • an option to hide the legend icon in the tooltip box
  • an optional function to complete overwrite the tooltip content

With that in place, I think we have achieved completely customizable tooltips!

I might have some additional enhancement proposals for L.Graph, but I'll open separate issues for that when I have explored/formalized my thoughts on that.

Thanks for spending time on this!

@sfairgrieve
Copy link
Contributor

Thanks for validating. I'll plan to add those additional features in the next few days. Any feedback/improvements you have for L.Graph will be much appreciated.

@sfairgrieve
Copy link
Contributor

@joakimkejser I pushed some updates to the feature/custom-tooltips branch, if you want to check it out. This should address #104 and #105.

For #104:

Under tooltipOptions you can include a getTooltip function that takes the record, legendDetails, and layerOptions and returns an L.DivIcon instance.

For #105:

Under tooltipOptions you can include a hideLegendBox property with a value of true to hide the legend box.

Let me know what you think when you get a chance.

@joakimkejser
Copy link
Contributor Author

Hi @sfairgrieve ,
Thanks for you continued effort to improve the tooltips.

After remembering to update the css, the hideLegendBox property in the tooltipOptions on the 'feature/custom-tooltips' branch works exactly as expected, solving #105.

I will test and explore the getTooltip function later.

Cheers.

@joakimkejser
Copy link
Contributor Author

Hi @sfairgrieve ,

The getTooltip in the tooltipOptions function works, and does indeed overwrite the complete tooltip! This is great!

I discovered a possible side effect of the changes. Using L.PieChartDataLayer (or any ChartDataLayer i assume), I now how to specify showLegendTooltips: true to get tooltips. This is a change from before, but it makes sense as L.ChartDataLayer specifies this as default:

options: {
            showLegendTooltips: false
        },

Furthermore, I seem to have two additional issues:

  • I cannot get the tooltip to display the values from the "slices" of the PieChartMarker like it did previously. When forcing it to show tooltips, I just get the tooltip box with a title and legendBox
  • The mouseOver highlighting seems to have changed. When I mouseover one of my PieCharts, I get a different styling that is not reset completely after mouseout, leaving me with a weight on the whole PieChart (not just the slice) where there previously was weight=0.

I can fix the mouserOver/Out issues by adding:

      setHighlight: function(options) {
        return options;
      },
      unsetHighlight: function(options) {
        return options;
      }

but then I lose the nice exaggerations of the slices on hover.

Can you replicate these issues or is it just me? I haven't had the time to read through the code thoroughly, so I'm sorry if I've missed something obvious.

@sfairgrieve
Copy link
Contributor

Thanks for testing this out! I'll investigate and see if I encounter the same issues.

@sfairgrieve
Copy link
Contributor

I see the issue. This is a side effect of removing the if statement around _bindMouseEvents a few commits back. Before, if showLegendTooltips was false, the DataLayer would not call _bindMouseEvents on child layers. Now, _bindMouseEvents is being called whether showLegendTooltips is true or false, which is preventing the child ChartMarker from getting that hover event. I'll try to put in a fix for this soon. The other issue this brings up is that ChartMarkers have their own tooltips, which the DataLayer settings don't affect - need to think about a way of fixing this/allowing customization that doesn't require duplicating too much code.

@joakimkejser
Copy link
Contributor Author

@sfairgrieve, your recent changes in the feature/custom-tooltips branch seems to have solved my issues. I'm aware that there still might be some customization issues - haven't checked any of that.

@vladislav-svetailo
Copy link

vladislav-svetailo commented Mar 18, 2021

No one of methods, which described above, didn't work for me. I didn't meet other explanation or any option in docs. In code I even didn't meet those options, which mentioned above. So, I put some changes in PieChartMarker section in leaflet-dvf.markers.js:

if(options.showTooltipOnChartPart==true) { this._bindMouseEvents(bar); }

and set option options.showTooltipOnChartPart to false. This helps me. Maybe I didn't understand something? I used version 1.0dev.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants