Skip to content

Conversation

@Indigo744
Copy link
Collaborator

I identified a regression with both PR #87 and #85
Function createLegends() shall be called only if new legends elements are given.
Otherwise, it is not possible to use the opt.setLegendElemsState option (since legend is always redrawn).

Functino `createLegends()` shall be called only if new legends elements are given.
Otherwise, it is not possible to use the `opt.setLegendElemsState` option (since legend is always redrawn).
@neveldo
Copy link
Owner

neveldo commented Nov 3, 2015

Hello,

I agree that createLegends() should be called only if new legends elements are given in the parameter, and that the "update legends" part should be surrounded by the condition :

if (typeof updatedOptions != "undefined" && typeof updatedOptions.legend === "object") {

But, regarding the 'setLegendElemsState' behaviour :

            // Set showlegendElems variable
            // If not defined, or badly defined, revert to TRUE
            if (typeof opt != "undefined" && typeof opt.setLegendElemsState === "string") {
                showlegendElems = (opt.setLegendElemsState === "hide") ? false : true;
            }

            // Hide/Show all elements based on showlegendElems
            //      Toggle (i.e. click) only if:
            //          - slice legend is shown AND we want to hide
            //          - slice legend is hidden AND we want to show
            $("[data-type='elem']", $(this)).each(function() {
                if (($(this).attr('data-hidden') === "0" && showlegendElems === false) || 
                    ($(this).attr('data-hidden') === "1" && showlegendElems === true)) {
                    // Toggle state of element by clicking
                    $(this).trigger('click', false);
                }
            });

Why not simply move this part of code at the end of the event ? It will allow users to update map legends AND to initialize them with a hidden state at the same time. And it will be unnecessary to check for "updateLegend === false" in order to change the legend elements state. What do you think about it ?

@Indigo744
Copy link
Collaborator Author

Really good idea! I was so focus on putting this call at the beginning, I forgot I could just move it around :-)

@neveldo
Copy link
Owner

neveldo commented Nov 4, 2015

Thank you for the code update, I merge your pull request !

neveldo added a commit that referenced this pull request Nov 4, 2015
Fix update event when hide/show element
@neveldo neveldo merged commit 403f2bd into neveldo:master Nov 4, 2015
@Indigo744 Indigo744 deleted the Indigo744-fix-update-legend branch November 4, 2015 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants