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

Duplicate Layers on the LayersControl. #135

Closed
isccarrasco opened this issue Oct 27, 2016 · 14 comments
Closed

Duplicate Layers on the LayersControl. #135

isccarrasco opened this issue Oct 27, 2016 · 14 comments

Comments

@isccarrasco
Copy link

Hi @mstahv
Firstly, thanks for your great job with the add-on.

I'm having a little problem using the add-on.

I'm creating and adding a list of LWmsLayer on the map using the addOverLay method on the init() of my UI class, all the layers are loaded correctly.

But, i added a button to create and add a choropleth layer on the map after init load, the problem are present when i click the button:

On the server side, the LayersControl have the list with the correct number of layers.

screen-2016

On the client side, when i display the layer contro, when i do this, the list of layer on the LayersControl on the client side duplicate the records of layers.

screen-2017

How can i prevent this issue?

@mstahv
Copy link
Owner

mstahv commented Nov 1, 2016

Hi Mario,

That looks to me that it doesn't seam to accommodate more room for the Layers control once it has already been shown with a bit shorter layer list. I'm not 100% sure but I'd suspect this is an issue in underlaying Leaflet JS.

You could try repeating the issue with plain LeafletJS and report the issue there. Alternatively, you could take another approach to layer selection, which I have found out to work pretty well. It is pretty easy to use core Vaadin UI components to build a layer selection as well, e.g. using option group or Table. You can then place the selector UI where ever you want but you can of course still put it e.g. on top of the LMap component using AbsoluteLayout.

cheers,
matti

@isccarrasco
Copy link
Author

Thanks @mstahv ...

For now, we are implementing the layer selector using another component (ComboBox), as you say, this is a good approach.

I'll try to reproduce the issue using Leaflet JS library, for check if this issue are on the JS library.

Thanks again.

PD. any information that i find, i´ll post it here.

@fkeusch
Copy link

fkeusch commented Nov 8, 2016

I just upgraded to 1.0.0 and I have the same issue. When create the LMap I add baseLayers like this:
lmap.addBaseLayer(gsatellite, "Google Maps satellite");
lmap.addBaseLayer(ghybrid, "Google Maps hybrid");

Then later when adding other wms layers the base layer duplicte somehow.
@isccarrasco did you already find a solution for this?
Is this confirmed to be a Leaflet JS issue?

In the earlier version we dis this (remove layer from controly and lmap)
LLayers controls = lmap.getLayersControl();
controls.removeLayer(currentWmsLayer);
lmap.removeLayer(currentWmsLayer);
-> with 1.0.0 this creates duplicates

Now I can do only this:
lmap.removeLayer(currentWmsLayer);
-> this seems to work for adding/removing this layer

but I still have the issue with the base layers

Thanks

@mstahv
Copy link
Owner

mstahv commented Nov 8, 2016

Could somebody add a reduced test case (here or a preferably as a pull request to projects tests)?

@fkeusch
Copy link

fkeusch commented Nov 8, 2016

@mstahv Yes. I'll try to add a very simple test case today or tomorrow. Thanks

@fkeusch
Copy link

fkeusch commented Nov 8, 2016

Sorry about my crappy code. But this simple example shows the problem (just copy it into a UI or View)

  1. When starting only the base layer 1 & 2 are in the control section.
  2. click on remove/add: adds a wms layer to the map as overlay. But then base layer 1 & 2 are duplicated and appear twice
  3. click on remove/add: remove wms layer and add a new wms layer. But then base layer 1 & 2 apear now 3 times

-> By just adding or removing layers the base layers get duplicated

I'm not sure weather this is a leaflet issue or a v-leaflet issue.

image

   VerticalLayout lmapContainer = new VerticalLayout();
    lmapContainer.setMargin(true);
    // create leaflet map
    lmap = new LMap();
    lmap.setCenter(40, 8);
    lmap.setWidth("500px");
    lmap.setHeight("400px");

    lmapContainer.addComponent(lmap);

    LOpenStreetMapLayer osm1 = new LOpenStreetMapLayer();
    osm1.setActive(false);
    lmap.addBaseLayer(osm1, "Base Layer 1");
    LOpenStreetMapLayer osm2 = new LOpenStreetMapLayer();
    osm2.setActive(true);
    lmap.addBaseLayer(osm2, "Base Layer 2");

    // adds and removes a wms layer
    // BUG: after removing/adding the base layer is duplicated
    Button wmsLayerRemoveAddButton = new Button("RemoveAdd");
    wmsLayerRemoveAddButton.addClickListener(new Button.ClickListener() {

        @Override
        public void buttonClick(ClickEvent event) {

            if (currentWmsLayer != null) {
                // this does not work anymore -> duplicates
                // LLayers controls = lmap.getLayersControl();
                // controls.removeLayer(currentWmsLayer);
                lmap.removeLayer(currentWmsLayer);
            }

            LWmsLayer result = new LWmsLayer();
            result.setFormat("image/png");
            result.setUrl("not/working/url/to/your/geoserver");
            result.setLayers("layerselection");
            currentWmsLayer = result;

            lmap.addOverlay(currentWmsLayer, "layer-"+System.currentTimeMillis());
        }
    });
    lmapContainer.addComponent(wmsLayerRemoveAddButton);
    addComponent(lmapContainer);

@octavm
Copy link
Contributor

octavm commented Nov 8, 2016

I can confirm I've experienced the same problem in the past too.
Layers in the Control were getting duplicated when I was adding an overlay after the Control has been already initialized and displayed on the map.

@fkeusch
Copy link

fkeusch commented Nov 8, 2016

Just tested a bit more. Adding overlays seems to duplicate all existing overlays somehow. If you my code to remove the currentWmsLayer all existing base layers and overlays (in my case wms layer) will be duplicated each time adding a new wmsLayer.

image

fkeusch pushed a commit to fkeusch/v-leaflet that referenced this issue Nov 8, 2016
@fkeusch
Copy link

fkeusch commented Nov 8, 2016

Nice way to run tests with UIRunner. I added a test case to easily check this issue.

@isccarrasco
Copy link
Author

Hi again :).

I tested the Leaflet JS adding layers on-demand (base layer and overlay layer), and the layer control don't duplicate the list of layer.

Here the example.

After click on the buttons, this es result.

captura de pantalla 2016-11-08 a la s 12 04 15

Regards.

@isccarrasco
Copy link
Author

Hi @fkeusch ...

For now, to avoid the duplicated layers, we are loading the layers with setActive(false) value

But, in the next release of our project, we thinking to list the layers on a grid and not use the layer control... is a good idea from @mstahv .

Regards.

@fkeusch
Copy link

fkeusch commented Nov 8, 2016

I think I found the bug. Debugging with chrome developer tools and setting breakpoints in all addOverlay() methods I actually tracked it down to this method in Java code:

LeafletLayersConnector.doStateChange(StateChangeEvent stateChangeEvent) {...}

I think this method calls addBaseLayer() and addOverlay() for each existing baseLayer and overlay which results in duplicates.

Can you verify this?

@fkeusch
Copy link

fkeusch commented Nov 8, 2016

The solution i see is to only add layers (with addBaseLayer() and addOverlay()) when they are not already added.

I'm not that deep into vaadin clientside development.

I tried to enhance the LayerControlInfo.java with a boolen value 'added' and then in LeafletLayersConnector.doStateChange() only add the layer when it's state is !added, and then set added = true. Somehow the state change (added = true) is not propagated back and it doesn't work.

Another solution I found is to always remove the layer before adding. Just call 'getControl().removeLayer(layerConnector.getLayer());' before adding the layer. This removes all layers from the control and then adds all layers to the control. This worked with my test case.
But I cannot see weather there are drawbacks of this.

mstahv added a commit that referenced this issue Nov 9, 2016
@mstahv mstahv closed this as completed in c2c1b8a Nov 9, 2016
@mstahv
Copy link
Owner

mstahv commented Nov 9, 2016

Thanks dudes, will cut a release soon!

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

No branches or pull requests

4 participants