rungwiroon / BlazorGoogleMaps

Blazor interop for GoogleMap library
MIT License
309 stars 99 forks source link

OnClick Events Not Working on Added Controls in v3.3.1 #298

Closed cruiserkernan closed 7 months ago

cruiserkernan commented 7 months ago

Issue: OnClick Events Not Working on Added Controls in BlazorGoogleMaps v3.3.1

Description

In BlazorGoogleMaps version 3.3.1, a bug was introduced affecting the onclick events on added controls to the map. This issue arose when support for removing controls was added, which included cloning the control HTML element. This cloning process leads to the loss of some data necessary for Blazor to receive the onclick events.

Bug Introduction

Current code

In objectManager.js createObject addControl, removeControl, removeControls

Proposed Solution

I suggest changes to the addControl, removeControl, and removeControls methods in objectManager.js:

createObject: function (args) {
    mapObjects = mapObjects || [];
    controlParents = {}; // Adds an object to store references to the original parent of the controls.
...
addControls(args) {
    let map = mapObjects[args[0]];
    let elem = args[2];
    if(!elem) return;
    let parentElement = elem.parentElement;
    controlParents[elem.id] = parentElement;

    //I know i am lazy. Two quotes appear after serialization
    let position = getGooglePositionFromString(args[1].replace(""", "").replace(""", ""));

    // check if the control already exists
    var controls = map.controls[position].getArray();
    for (var i = 0; i < controls.length; i++) {
        if (controls[i].id === elem.id) {
            return;
        }
    }
    map.controls[position].push(elem);
},
removeControl(args) {
    let map = mapObjects[args[0]];
    let elem = args[2];
    let position = getGooglePositionFromString(args[1].replace(""", "").replace(""", ""));

    var controls = map.controls[position].getArray();
    for (var i = 0; i < controls.length; i++) {
        if (controls[i].id === elem.id) {
            let control = map.controls[position].removeAt(i);
            let parent = controlParents[control.id];
            if (parent) {
                parent.appendChild(control);
                control.style.visibility = "hidden";
            }
            controlParents[control.id] = null;
            return;
        }
    }
},
removeControls(args) {
    let map = mapObjects[args[0]];
    let position = getGooglePositionFromString(args[1].replace(""", "").replace(""", ""));

    while (map.controls[position].length > 0) {
        let control = map.controls[position].pop();
        let parent = controlParents[control.id];
        if (parent) {
            parent.appendChild(control);
            control.style.visibility = "hidden";
        }
        controlParents[control.id] = null;
    }
}

With these changes the original element will not be cloned it will just be switching place in the DOM, keeping the needed references for blazor interop to work.

Another solution might be to just hide the control element in the map instead of removing it.

What do you think?

valentasm1 commented 7 months ago

Could you provide code to reproduce bug? About code. Without going to details looks good. What controlObjects usage? i dont see it. Also if you gonna put into separate array then you need to cleanup on dispose.

cruiserkernan commented 7 months ago

It should be controlParents, which is the object to store the original parent element of each control. I updated the proposed solution. I will also add the code in dispose. I will provide code to reproduce the bug.

I could make a pull request if you want to, with some examples of events working in the MapLegendPage , and tested?

valentasm1 commented 7 months ago

PR would help.

Enritski commented 7 months ago

I can confirm that this bug exists. I was about to submit a report for this very thing, then came across this one. In my case, I have a control placed on the map that contain a couple of the "switch" controls from Blazorise. Since upgrading to v 3.3.1 of BlazorGoogleMaps, those controls still work (i.e., their CheckedChanged handler fires when the switch is clicked), but their visual state does not update - so you can't see whether the switch is on or off.

valentasm1 commented 7 months ago

Could you provide simple demo to reproduce?

valentasm1 commented 7 months ago

Than you for your time https://www.nuget.org/packages/BlazorGoogleMaps/4.0.1

Enritski commented 7 months ago

Works great! Thank you both!