search

neveldo / jQuery-Mapael

jQuery plugin based on raphael.js that allows you to display dynamic vector maps
https://www.vincentbroute.fr/mapael/
MIT License
1.01k stars 195 forks source link

Fix 'clicked' legend element behavior #354

Closed neveldo closed 7 years ago

neveldo commented 7 years ago

Fix https://github.com/neveldo/jQuery-Mapael/issues/349 .

The 'clicked' option is handled through a handleClickOnLegendElem() call within drawLegend() function.

However, the legend creation precede the areas and plots creation, so the 'clicked' option can't be handled properly. (however, it's weird, because it seems to work with areas, but not with plots ...)

I have moved the "Handle map size" part (which include legends creation) right after the creation of the areas, plots & links. It seems there is no side effect.

Before : http://jsfiddle.net/neveldo/sckgyLyr/ After : http://jsfiddle.net/neveldo/adbr8u7L/

Indigo744 commented 7 years ago

There is maybe a potential side effect: in beforeInit hook, user may expect the map to be already properly sized. I'm not really sure of the implication of this...

So maybe we could only move the createLegends() call. But there is an issue with the initResponsiveSize()...

The best approach would then be to extract the code related to the clicked option in another function, and to call it afterwards...

What do you think? Maybe I'm thinking too much... :smile:

neveldo commented 7 years ago

Hello,

You are right, I didn't thought about the beforeInit init hook, but it seems to work well within the afterInit_extend_raphael_paper.html example for instance (I have juste replaced 'afterInit' hook by a 'beforeInit'. Moreover, in the version 1.1.0, the "Handle map size" part was already the last one in the initialisation process. But maybe this part have been moved for a particular reason ? I don't remember :(

Otherwise, as you said, we could extract the handle of 'clicked' option and move it after legends, areas, plots, and links creation. But I think it will be trickier to handle this option on legend updates for instance.

Indigo744 commented 7 years ago

I have a simple idea. May I commit to the branch to show you? In any case, we will revert if we want :smile:

Indigo744 commented 7 years ago

What do you think?

With this, now the createLegends() calls for areas and plots are in the same place in init and the issue is resolved.

neveldo commented 7 years ago

@Indigo744 , Of course, you are welcome ! (feel free to open a new branch if you prefer :) )

neveldo commented 7 years ago

Wow, you are quick, I will check it asap !

Indigo744 commented 7 years ago

Ahah sorry I went ahead! I wanted to do this before forgetting my idea 😅

neveldo commented 7 years ago

I have to admit that your solution is a lot better that mine, as it remove some useless complexity ! I approve it :)