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

Add/remove custom controls dynamically #138

Merged
merged 7 commits into from
Sep 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

<groupId>com.flowingcode.vaadin.addons</groupId>
<artifactId>google-maps</artifactId>
<version>2.1.1-SNAPSHOT</version>
<version>2.2.0-SNAPSHOT</version>
<name>Google Maps Addon</name>
<description>Integration of google-map for Vaadin platform</description>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,19 +42,23 @@
import elemental.json.JsonArray;
import elemental.json.JsonObject;
import elemental.json.JsonValue;
import java.util.ArrayList;
import java.util.ArrayList;
Comment on lines +45 to +46
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Remove duplicate import statements

Lines 45 and 46 both import java.util.ArrayList. This duplicate import is unnecessary and should be removed.

Apply this diff to remove the duplicate import:

-import java.util.ArrayList;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import java.util.ArrayList;
import java.util.ArrayList;
import java.util.ArrayList;

import java.util.List;
import java.util.concurrent.CompletableFuture;
import org.apache.commons.lang3.StringUtils;

@SuppressWarnings("serial")
@Tag("google-map")
@JsModule("@flowingcode/google-map/google-map.js")
@NpmPackage(value = "@flowingcode/google-map", version = "3.7.1")
@NpmPackage(value = "@flowingcode/google-map", version = "3.8.0")
@NpmPackage(value = "@googlemaps/markerclusterer", version = "2.0.8")
@JsModule("./googlemaps/geolocation.js")
public class GoogleMap extends Component implements HasSize {

private Integer trackLocationId = null;
private Integer trackLocationId = null;

private List<CustomControl> customControls = new ArrayList<CustomControl>();

/** Base map types supported by Google Maps. */
public enum MapType {
Expand Down Expand Up @@ -776,21 +780,53 @@ public void addCustomControls(CustomControl... customControls) {
}
this.getElement().setPropertyJson("customControls", jsonArray);
}

/**
* Sets the custom control buttons to be displayed in the map.
*
* @param customControls list of custom controls to add to the map
*/
public void setCustomControls(CustomControl... customControls) {
JsonArray jsonArray = Json.createArray();
for (int i = 0; i < customControls.length; i++) {
CustomControl customControl = customControls[i];
jsonArray.set(i, customControl.getJson(i));
customControl.getControlButton().getElement().setAttribute("slot", "customControlSlot_" + i);
this.getElement().appendChild(customControl.getControlButton().getElement());
}
this.getElement().setPropertyJson("customControls", jsonArray);
this.customControls.clear();
this.getElement().executeJs("this._removeCustomControls()").then((e) -> {
JsonArray jsonArray = Json.createArray();
for (int i = 0; i < customControls.length; i++) {
CustomControl customControl = customControls[i];
jsonArray.set(i, customControl.getJson(i));
customControl.getControlButton().getElement().setAttribute("slot", "customControlSlot_" + i);
this.getElement().appendChild(customControl.getControlButton().getElement());
this.customControls.add(customControl);
}
this.getElement().setPropertyJson("customControls", jsonArray);
});
}
paodb marked this conversation as resolved.
Show resolved Hide resolved
Comment on lines +790 to +802
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Potential concurrency issue when modifying customControls

In the setCustomControls method, this.customControls.clear() is called synchronously, whereas this.customControls.add(customControl) is called inside the asynchronous callback of executeJs(...).then(...). This could lead to an inconsistent state of customControls if other methods access it before the callback completes.

Consider moving this.customControls.clear() inside the then block to ensure that all modifications to customControls occur sequentially after the JavaScript execution. This adjustment will help maintain consistency in the state of customControls.

Apply this diff to adjust the code:

 public void setCustomControls(CustomControl... customControls) {
-  this.customControls.clear();
   this.getElement().executeJs("this._removeCustomControls()").then((e) -> {
+    this.customControls.clear();
     JsonArray jsonArray = Json.createArray();
     for (int i = 0; i < customControls.length; i++) {
       CustomControl customControl = customControls[i];
       jsonArray.set(i, customControl.getJson(i));
       customControl.getControlButton().getElement().setAttribute("slot", "customControlSlot_" + i);
       this.getElement().appendChild(customControl.getControlButton().getElement());
       this.customControls.add(customControl);
     }
     this.getElement().setPropertyJson("customControls", jsonArray);
   });    
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
this.customControls.clear();
this.getElement().executeJs("this._removeCustomControls()").then((e) -> {
JsonArray jsonArray = Json.createArray();
for (int i = 0; i < customControls.length; i++) {
CustomControl customControl = customControls[i];
jsonArray.set(i, customControl.getJson(i));
customControl.getControlButton().getElement().setAttribute("slot", "customControlSlot_" + i);
this.getElement().appendChild(customControl.getControlButton().getElement());
this.customControls.add(customControl);
}
this.getElement().setPropertyJson("customControls", jsonArray);
});
}
public void setCustomControls(CustomControl... customControls) {
this.getElement().executeJs("this._removeCustomControls()").then((e) -> {
this.customControls.clear();
JsonArray jsonArray = Json.createArray();
for (int i = 0; i < customControls.length; i++) {
CustomControl customControl = customControls[i];
jsonArray.set(i, customControl.getJson(i));
customControl.getControlButton().getElement().setAttribute("slot", "customControlSlot_" + i);
this.getElement().appendChild(customControl.getControlButton().getElement());
this.customControls.add(customControl);
}
this.getElement().setPropertyJson("customControls", jsonArray);
});
}


/**
* Adds a custom control to be displayed in the map.
*
* @param customControl the custom control to add to the map
*/
public void addCustomControl(CustomControl customControl) {
this.customControls.add(customControl);
this.setCustomControls(this.customControls.stream().toArray(CustomControl[]::new));
}

/**
* Removes a custom control added to the map.
*
* @param customControl the custom control to be removed
*/
public void removeCustomControl(CustomControl customControl) {
this.customControls.remove(customControl);
this.setCustomControls(this.customControls.stream().toArray(CustomControl[]::new));
}

Comment on lines +809 to +823
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Optimize addCustomControl and removeCustomControl methods to improve performance

Currently, addCustomControl and removeCustomControl invoke setCustomControls, which resets all custom controls. This approach can be inefficient, especially when adding or removing controls frequently.

Consider modifying addCustomControl and removeCustomControl to update the controls incrementally without resetting all existing controls. You can directly add or remove the specific control and update the map accordingly. This change will enhance performance and provide a smoother user experience.

/**
* Removes all custom controls added to the map.
*/
public void removeCustomControls() {
this.customControls.clear();
this.getElement().executeJs("this._removeCustomControls()");
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
@SuppressWarnings("serial")
@Tag("google-map-marker")
@JsModule("@flowingcode/google-map/google-map-marker.js")
@NpmPackage(value = "@flowingcode/google-map", version = "3.7.1")
@NpmPackage(value = "@flowingcode/google-map", version = "3.8.0")
@NpmPackage(value = "@googlemaps/markerclusterer", version = "2.0.8")
public class GoogleMapMarker extends Component {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
@SuppressWarnings("serial")
@Tag("google-map-point")
@JsModule("@flowingcode/google-map/google-map-point.js")
@NpmPackage(value = "@flowingcode/google-map", version = "3.7.1")
@NpmPackage(value = "@flowingcode/google-map", version = "3.8.0")
@NpmPackage(value = "@googlemaps/markerclusterer", version = "2.0.8")
public class GoogleMapPoint extends Component {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
@Tag("google-map-poly")
@JsModule("@flowingcode/google-map/google-map-poly.js")
@JsModule("@flowingcode/google-map/google-map-point.js")
@NpmPackage(value = "@flowingcode/google-map", version = "3.7.1")
@NpmPackage(value = "@flowingcode/google-map", version = "3.8.0")
@NpmPackage(value = "@googlemaps/markerclusterer", version = "2.0.8")
public abstract class GoogleMapPoly extends Component {

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
/*-
* #%L
* Google Maps Addon
* %%
* Copyright (C) 2020 - 2024 Flowing Code
* %%
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
* #L%
*/

package com.flowingcode.vaadin.addons.googlemaps;

import com.flowingcode.vaadin.addons.demo.DemoSource;
import com.flowingcode.vaadin.addons.googlemaps.GoogleMap.MapType;
import com.vaadin.flow.component.button.Button;
import com.vaadin.flow.component.dependency.CssImport;
import com.vaadin.flow.component.orderedlayout.HorizontalLayout;
import com.vaadin.flow.router.PageTitle;
import com.vaadin.flow.router.Route;

@PageTitle("Custom Controls Demo")
@DemoSource
@DemoSource(
value = "/src/test/resources/META-INF/resources/frontend/styles/google-maps/custom-controls-demo-styles.css",
caption = "custom-controls-demo-styles.css")
@Route(value = "googlemaps/custom-controls", layout = GooglemapsDemoView.class)
@CssImport("./styles/google-maps/custom-controls-demo-styles.css")
@SuppressWarnings("serial")
public class CustomControlsDemo extends AbstractGoogleMapsDemo {

@Override
protected void createGoogleMapsDemo(String apiKey) {
GoogleMap gmaps = new GoogleMap(apiKey, null, null);
gmaps.setMapType(MapType.ROADMAP);
gmaps.setSizeFull();
add(gmaps);

Button customControlButton1 = new Button("Custom Control 1");
customControlButton1.setClassName("custom-control-button");
CustomControl customControl1 =
new CustomControl(customControlButton1, ControlPosition.TOP_CENTER);
Button customControlButton2 = new Button("Custom Control 2");
customControlButton2.setClassName("custom-control-button");
CustomControl customControl2 =
new CustomControl(customControlButton2, ControlPosition.LEFT_CENTER);
gmaps.setCustomControls(customControl1, customControl2);

Button customControlButton3 = new Button("Custom Control 3");
customControlButton3.setClassName("custom-control-button");
CustomControl customControl3 =
new CustomControl(customControlButton3, ControlPosition.BOTTOM_CENTER);

Button addCustomControl3Button = createDemoButton("Add Custom Control 3");
Button removeCustomControl3Button = createDemoButton("Remove Custom Control 3");
Button removeAllCustomControlsButton = createDemoButton("Remove all controls");
Button resetButton = createDemoButton("Reset");

addCustomControl3Button.addClickListener(e -> {
gmaps.addCustomControl(customControl3);
removeCustomControl3Button.setEnabled(true); // hide-source
removeAllCustomControlsButton.setEnabled(true); // hide-source
});

removeCustomControl3Button.addClickListener(e -> {
gmaps.removeCustomControl(customControl3);
addCustomControl3Button.setEnabled(true); // hide-source
});
removeCustomControl3Button.setEnabled(false);

removeAllCustomControlsButton.addClickListener(e -> {
gmaps.removeCustomControls();
addCustomControl3Button.setEnabled(true); // hide-source
removeCustomControl3Button.setEnabled(false); // hide-source
resetButton.setEnabled(true); // hide-source
});

resetButton.addClickListener(e -> {
gmaps.setCustomControls(customControl1, customControl2);
removeAllCustomControlsButton.setEnabled(true); // hide-source
addCustomControl3Button.setEnabled(true); // hide-source
removeCustomControl3Button.setEnabled(false); // hide-source
});

add(new HorizontalLayout(addCustomControl3Button, removeCustomControl3Button,
removeAllCustomControlsButton, resetButton));
}
paodb marked this conversation as resolved.
Show resolved Hide resolved

private Button createDemoButton(String caption) {
Button button = new Button(caption);
button.setDisableOnClick(true);
return button;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ public GooglemapsDemoView() {
addDemo(CustomizedMarkerIconsDemo.class);
addDemo(TrackLocationDemo.class);
addDemo(StyleFeaturesDemo.class);
addDemo(CustomControlsDemo.class);
setSizeFull();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*-
* #%L
* Google Maps Addon
* %%
* Copyright (C) 2020 - 2024 Flowing Code
* %%
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
* #L%
*/

.custom-control-button {
background: white;
margin: 10px;
height: 40px;
color: black;
cursor: pointer;
font-family: Arial;
font-size: 18px;
border-radius: 0;
box-shadow: rgba(0, 0, 0, 0.3) 0px 1px 4px -1px;
}
Loading