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

Update trigger does not respect new cssClass #314

Closed jordanread closed 7 years ago

jordanread commented 7 years ago

While creating an area array, and passing it through to the update event, when new cssClasses are assigned, it is ignored.

Here is a proof of concept using the "Trigger an 'update' event for refreshing elements" example included.

Add a new css class: .updatedArea { fill: #00FF00; stroke: #FF0000; }

Modify the on click event: $('#refresh').on('click', function () {

            // Update some plots and areas attributes ...
            var updatedOptions = {'areas': {}};
            updatedOptions.areas["department-56"] = {
                tooltip: {
                    content: "Morbihan (56) (2)"
                },
                cssClass: "updatedArea",
                text: {content: "56 (2)"}
            };

            $(".mapcontainer").trigger('update', [{
                mapOptions: updatedOptions, 
                animDuration: 1000
            }]);
        });
neveldo commented 7 years ago

Hello @jordanread , you are right, the cssClass handling seems to be missing from the updateElem() function from Mapael. I will take a look ASAP (of course, feel free to contribute if you can)

Indigo744 commented 7 years ago

@neveldo can I look into it or are you already working on it? ;)

neveldo commented 7 years ago

@Indigo744 No, in fact I have planned to work on it these next days, so of course you can still take it if you want !

I quickly dived into the code, and it seems that the handling of the cssClass attribute is just missing from the updateElem() function.

jordanread commented 7 years ago

@Indigo744 I did notice that Raphael actually has a 'class' section under the attrs. That does in theory work, but only for a minute. Something else actually overrides it on the mapael update (I can see the flash on the screen). @neveldo I do intend to contribute a bit as I can. I've got a potential fix for passing an array to the legend, because it doesn't work as it stands. I will refactor and do a pull request for that one. I'm a little gun shy on this one, because what I am using it for is not quite the use case I think (dealing with much smaller areas).

Indigo744 commented 7 years ago

See PR #318 that should fix your issue

neveldo commented 7 years ago

Hi @jordanread,

I have just merged the fix provided by @Indigo744 on the master branch. Could you check it into your use case and give us your feedback ?

jordanread commented 7 years ago

@neveldo I pulled down the latest, and confirmed it works as expected. Thank you.

neveldo commented 7 years ago

Nice, thanks @Indigo744 for the fix !