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

CustomControl Buttons are rePostioned! cant have different positions of multiple buttons! #127

Closed
JariHanah opened this issue May 15, 2024 · 8 comments · Fixed by #129
Closed
Assignees

Comments

@JariHanah
Copy link

Describe the bug

I added multiple Custom Buttons inside maps using CustomControl,
All added buttons would be repositioned to the position of the last Button inserted.

Expected behavior

Each button should follow how we position it in code!
addCustomControls(new CustomControl(button1, ControlPosition.BOTTOM_CENTER));
addCustomControls(new CustomControl(button2, ControlPosition.TOP_CENTER));

button1 should be positioned bottom,
button2 should be positioned TOP

Minimal reproducible example

gmaps.addCustomControls(new CustomControl(button2, ControlPosition.TOP_CENTER));
gmaps.addCustomControls(new CustomControl(button1, ControlPosition.BOTTOM_CENTER));

Add-on Version

1.12.0

Vaadin Version

24

Additional information

image

@github-project-automation github-project-automation bot moved this to Inbox (needs triage) in Flowing Code Addons May 15, 2024
@paodb
Copy link
Member

paodb commented May 15, 2024

Hello, if you change your code to

gmaps.addCustomControls(new CustomControl(button2, ControlPosition.TOP_CENTER),new CustomControl(button1, ControlPosition.BOTTOM_CENTER)); 

it works as expected.

We understand that calling the method "addCustomControls" can be confusing so we're discussing to make a change in order to have a "setCustomControls" instead, to set the list of custom controls.

@paodb paodb moved this from Inbox (needs triage) to To Do in Flowing Code Addons May 15, 2024
@paodb paodb self-assigned this May 15, 2024
@paodb paodb moved this from To Do to In Progress in Flowing Code Addons May 15, 2024
@JariHanah
Copy link
Author

I will test your solution, however,
My use case involves adding buttons dynamically,
Also, there is no way to remove a Custom Control.

@paodb
Copy link
Member

paodb commented May 16, 2024

My use case involves adding buttons dynamically,
Also, there is no way to remove a Custom Control.

Those requirements will require some investigation. I will create a new issue in order to address them.

I will move forward with the update on the original reported issue.

@paodb
Copy link
Member

paodb commented May 16, 2024

Here's the new issue #128. @JariHanah could you add there any additional information about your use case and expected behavior? Thanks.

@JariHanah
Copy link
Author

I think a setCustomButtons and removeCuustomButtons would solve my issues,
Mayby its only one method a setCustomButtons the can be given an empty set to remove all customs, or substitute the buttons

Hello, if you change your code to

gmaps.addCustomControls(new CustomControl(button2, ControlPosition.TOP_CENTER),new CustomControl(button1, ControlPosition.BOTTOM_CENTER)); 

it works as expected.

We understand that calling the method "addCustomControls" can be confusing so we're discussing to make a change in order to have a "setCustomControls" instead, to set the list of custom controls.

@JariHanah
Copy link
Author

what I did to solve the problem, extending the CustomControl class,
remove all custom elements when I try to add a new button,
here is the setCustomButtons,


static class CustomControl2 extends CustomControl {
        CustomControl2(Button b, ControlPosition pos) {
            super(b, pos);
        }

        public JsonObject getJson(int i) {
            return super.getJson(i);
        }
    }

List<Element[]> elements = new ArrayList<>();

    public void setControls(CustomControl2[] customControls) {
        JsonArray jsonArray = Json.createArray();
        elements.forEach(element -> {
            Arrays.stream(element).forEach(element1 -> {
                if (element1 != null) {
                    gmaps.getElement().removeChild(element1);
                }
            });

        });
        elements.clear();
        for (int i = 0; i < customControls.length; ++i) {
            CustomControl2 customControl = customControls[i];
            jsonArray.set(i, customControl.getJson(i));
            customControl.getControlButton().getElement().setAttribute("slot", "customControlSlot_" + i);
            Element p = gmaps.getElement();
            Element[] e = new Element[]{customControl.getControlButton().getElement()};
            gmaps.getElement().appendChild(e);
            elements.add(e);
        }
        gmaps.getElement().setPropertyJson("customControls", jsonArray);
    }

javier-godoy pushed a commit that referenced this issue May 21, 2024
Use setCustomControls instead.

Close #127
@github-project-automation github-project-automation bot moved this from In Progress to Done in Flowing Code Addons May 21, 2024
@JariHanah
Copy link
Author

So I tried the new method of setCustomControls for the latest version of 1.13.0
each time a setCustomControls, the original buttons don't get removed.

Check Sample Image,
image

it still acts like an addCustomControls which is deprecated.

what was this issue supposed to solve?

My Prevoius code shows how my setCustomControls work by removing the previous controls, then applying a new set of controls.
Even though seems the positions of controls seems to shift after some use (not sure why) but overall, its working.

@paodb
Copy link
Member

paodb commented Jun 1, 2024

Hello @JariHanah as mentioned in this comment #127 (comment) this ticket would only address the update of addCustomControls to setCustomcontrols without any other change, which fixes the original reported issue. That comment also mentioned that removing or adding controls dynamically is something that needs investigation and a separate issue was created for that #128. We didn't have the change to work on that yet. Any new update regarding removing/adding dynamically will be done in that other issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging a pull request may close this issue.

2 participants