Skip to content

Commit 4cd7d53

Browse files
authored
fix: auto-attach Popover based on helper annotation (#8110)
Introduces a new internal Java annotation interface to help Popover decide where to attach itself when the component is auto added to the UI. Adds the `@ModalComponent` annotation to the following components: - Dialog - ConfirmDialog - LoginOverlay When the Popover target's element is inside any of the components above, the component will be placed in the same parent as the target's to allow events to be listened from the Popover content.
1 parent 153577a commit 4cd7d53

File tree

6 files changed

+239
-1
lines changed
  • vaadin-confirm-dialog-flow-parent/vaadin-confirm-dialog-flow/src/main/java/com/vaadin/flow/component/confirmdialog
  • vaadin-dialog-flow-parent/vaadin-dialog-flow/src/main/java/com/vaadin/flow/component/dialog
  • vaadin-flow-components-shared-parent/vaadin-flow-components-base/src/main/java/com/vaadin/flow/component/shared/internal
  • vaadin-login-flow-parent/vaadin-login-flow/src/main/java/com/vaadin/flow/component/login
  • vaadin-popover-flow-parent/vaadin-popover-flow/src

6 files changed

+239
-1
lines changed

vaadin-confirm-dialog-flow-parent/vaadin-confirm-dialog-flow/src/main/java/com/vaadin/flow/component/confirmdialog/ConfirmDialog.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import com.vaadin.flow.component.dependency.JsModule;
3333
import com.vaadin.flow.component.dependency.NpmPackage;
3434
import com.vaadin.flow.component.shared.SlotUtils;
35+
import com.vaadin.flow.component.shared.internal.ModalRoot;
3536
import com.vaadin.flow.component.shared.internal.OverlayAutoAddController;
3637
import com.vaadin.flow.dom.Style;
3738
import com.vaadin.flow.shared.Registration;
@@ -62,6 +63,7 @@
6263
@Tag("vaadin-confirm-dialog")
6364
@NpmPackage(value = "@vaadin/confirm-dialog", version = "25.0.0-alpha20")
6465
@JsModule("@vaadin/confirm-dialog/src/vaadin-confirm-dialog.js")
66+
@ModalRoot
6567
public class ConfirmDialog extends Component
6668
implements HasComponents, HasSize, HasStyle {
6769

vaadin-dialog-flow-parent/vaadin-dialog-flow/src/main/java/com/vaadin/flow/component/dialog/Dialog.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
import com.vaadin.flow.component.dependency.JsModule;
4040
import com.vaadin.flow.component.dependency.NpmPackage;
4141
import com.vaadin.flow.component.shared.HasThemeVariant;
42+
import com.vaadin.flow.component.shared.internal.ModalRoot;
4243
import com.vaadin.flow.component.shared.internal.OverlayAutoAddController;
4344
import com.vaadin.flow.dom.Element;
4445
import com.vaadin.flow.dom.ElementConstants;
@@ -78,6 +79,7 @@
7879
@NpmPackage(value = "@vaadin/dialog", version = "25.0.0-alpha20")
7980
@JsModule("@vaadin/dialog/src/vaadin-dialog.js")
8081
@JsModule("./flow-component-renderer.js")
82+
@ModalRoot
8183
public class Dialog extends Component implements HasComponents, HasSize,
8284
HasStyle, HasThemeVariant<DialogVariant> {
8385

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
/*
2+
* Copyright 2000-2025 Vaadin Ltd.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License"); you may not
5+
* use this file except in compliance with the License. You may obtain a copy of
6+
* the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
12+
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
13+
* License for the specific language governing permissions and limitations under
14+
* the License.
15+
*/
16+
package com.vaadin.flow.component.shared.internal;
17+
18+
import java.lang.annotation.ElementType;
19+
import java.lang.annotation.Retention;
20+
import java.lang.annotation.RetentionPolicy;
21+
import java.lang.annotation.Target;
22+
23+
/**
24+
* The purpose of this annotation is to help {@code Popover} decide where to
25+
* auto-attach itself based on the target component.
26+
* <p>
27+
* Normally, {@code Popover} attaches to the UI's root element, using the
28+
* {@code UI#addToModalComponent} method. But that is problematic when the
29+
* target component is attached inside an element with modality, such as
30+
* {@code Dialog}. This prevents any events in the client-side fired from the
31+
* {@code Popover} component or any of its children from being listened to in
32+
* the server-side.
33+
* <p>
34+
* To solve this, {@code Popover} tries to find the closest parent component
35+
* that has this annotation, and attaches to that component instead of the UI
36+
* root.
37+
* <p>
38+
* <strong> Internal use only. May be renamed or removed in a future release.
39+
* </strong>
40+
*/
41+
@Retention(RetentionPolicy.RUNTIME)
42+
@Target(ElementType.TYPE)
43+
public @interface ModalRoot {
44+
45+
/**
46+
* The slot to use when attaching to the modal root. If empty, no slot
47+
* attribute is set.
48+
*/
49+
String slot() default "";
50+
}

vaadin-login-flow-parent/vaadin-login-flow/src/main/java/com/vaadin/flow/component/login/LoginOverlay.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import com.vaadin.flow.component.dependency.JsModule;
2828
import com.vaadin.flow.component.dependency.NpmPackage;
2929
import com.vaadin.flow.component.shared.SlotUtils;
30+
import com.vaadin.flow.component.shared.internal.ModalRoot;
3031
import com.vaadin.flow.component.shared.internal.OverlayAutoAddController;
3132
import com.vaadin.flow.dom.Element;
3233
import com.vaadin.flow.dom.Style;
@@ -48,6 +49,7 @@
4849
@Tag("vaadin-login-overlay")
4950
@NpmPackage(value = "@vaadin/login", version = "25.0.0-alpha20")
5051
@JsModule("@vaadin/login/src/vaadin-login-overlay.js")
52+
@ModalRoot(slot = "footer")
5153
public class LoginOverlay extends AbstractLogin implements HasStyle {
5254

5355
private Component title;

vaadin-popover-flow-parent/vaadin-popover-flow/src/main/java/com/vaadin/flow/component/popover/Popover.java

Lines changed: 61 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import com.vaadin.flow.component.dependency.JsModule;
3434
import com.vaadin.flow.component.dependency.NpmPackage;
3535
import com.vaadin.flow.component.shared.HasThemeVariant;
36+
import com.vaadin.flow.component.shared.internal.ModalRoot;
3637
import com.vaadin.flow.dom.Style;
3738
import com.vaadin.flow.internal.JacksonUtils;
3839
import com.vaadin.flow.server.VaadinService;
@@ -729,17 +730,76 @@ private void removeFromUiIfAutoAdded() {
729730
}
730731

731732
private void onTargetAttach(UI ui) {
733+
732734
if (target != null) {
733735
if (getElement().getNode().getParent() == null) {
734736
// Remove the popover from its current state tree
735737
getElement().removeFromTree(false);
736-
ui.addToModalComponent(this);
738+
getModalParentComponent().ifPresentOrElse(modalParent -> {
739+
updateSlotAttribute(modalParent);
740+
741+
if (modalParent instanceof HasComponents) {
742+
((HasComponents) modalParent).add(this);
743+
} else {
744+
modalParent.getElement().appendChild(getElement());
745+
}
746+
}, () -> {
747+
ui.addToModalComponent(this);
748+
});
749+
737750
autoAddedToTheUi = true;
738751
}
739752
getElement().executeJs("this.target = $0", target.getElement());
740753
}
741754
}
742755

756+
/**
757+
* Finds the closest parent component that is annotated with
758+
* {@link ModalRoot}, if any.
759+
*
760+
* @return an optional with the closest modal parent component, or an empty
761+
* optional if none found
762+
*/
763+
private Optional<Component> getModalParentComponent() {
764+
var parent = target.getParent();
765+
while (parent.isPresent()) {
766+
var parentComponent = parent.get();
767+
if (parentComponent.getClass()
768+
.isAnnotationPresent(ModalRoot.class)) {
769+
return parent;
770+
}
771+
parent = parentComponent.getParent();
772+
}
773+
return Optional.empty();
774+
}
775+
776+
/**
777+
* Updates the {@code slot} attribute based on the target or its ancestors
778+
* up to the modal parent.
779+
* <p>
780+
* It starts by removing any existing {@code slot} attribute from the
781+
* popover. Then, it gets the {@link ModalRoot} annotation from the modal
782+
* parent component to determine the appropriate slot value. If the modal
783+
* parent defines a value for the {@link ModalRoot#slot()} property, it sets
784+
* the {@code slot} attribute on the popover element.
785+
*
786+
* <p>
787+
* This ensures that the popover is rendered in the correct slot when used
788+
* inside a modal component.
789+
*
790+
* @param modalParent
791+
* the modal parent component
792+
*/
793+
private void updateSlotAttribute(Component modalParent) {
794+
getElement().removeAttribute("slot");
795+
var annotation = modalParent.getClass().getAnnotation(ModalRoot.class);
796+
797+
var slotValue = annotation.slot();
798+
if (slotValue != null && !slotValue.isEmpty()) {
799+
getElement().setAttribute("slot", slotValue);
800+
}
801+
}
802+
743803
/**
744804
* Gets the target component of this popover, or {@code null} if it doesn't
745805
* have a target.

vaadin-popover-flow-parent/vaadin-popover-flow/src/test/java/com/vaadin/flow/component/popover/PopoverAutoAddTest.java

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,12 @@
2121
import org.junit.Test;
2222
import org.mockito.Mockito;
2323

24+
import com.vaadin.flow.component.Component;
25+
import com.vaadin.flow.component.HasComponents;
26+
import com.vaadin.flow.component.Tag;
2427
import com.vaadin.flow.component.UI;
2528
import com.vaadin.flow.component.html.Div;
29+
import com.vaadin.flow.component.shared.internal.ModalRoot;
2630
import com.vaadin.flow.server.VaadinSession;
2731

2832
/**
@@ -184,9 +188,127 @@ public void setTarget_openModal_popoverIsAttachedToUi() {
184188
Assert.assertEquals(ui, popover.getParent().orElseThrow());
185189
}
186190

191+
@Test
192+
public void popoverWithTargetInModalComponent_popoverAttachedToModal() {
193+
var modal = new TestModalComponent();
194+
ui.add(modal);
195+
var target = new Div();
196+
modal.getElement().appendChild(target.getElement());
197+
var popover = new Popover();
198+
popover.setTarget(target);
199+
fakeClientResponse();
200+
Assert.assertEquals("Popover should be attached to modal", modal,
201+
popover.getParent().orElse(null));
202+
}
203+
204+
@Test
205+
public void popoverWithTargetInModalComponent_targetRemoved_popoverDetached() {
206+
var modal = new TestModalComponent();
207+
ui.add(modal);
208+
var target = new Div();
209+
modal.getElement().appendChild(target.getElement());
210+
var popover = new Popover();
211+
popover.setTarget(target);
212+
fakeClientResponse();
213+
214+
target.removeFromParent();
215+
fakeClientResponse();
216+
Assert.assertFalse("Popover should be detached",
217+
popover.getParent().isPresent());
218+
}
219+
220+
@Test
221+
public void popoverWithTargetInModalContainer_popoverAttachedToModal() {
222+
var modal = new TestModalContainer();
223+
ui.add(modal);
224+
var target = new Div();
225+
modal.add(target);
226+
var popover = new Popover();
227+
popover.setTarget(target);
228+
fakeClientResponse();
229+
Assert.assertEquals("Popover should be attached to modal", modal,
230+
popover.getParent().orElse(null));
231+
}
232+
233+
@Test
234+
public void popoverWithTargetInModalContainer_targetRemoved_popoverDetached() {
235+
var modal = new TestModalContainer();
236+
ui.add(modal);
237+
var target = new Div();
238+
modal.add(target);
239+
var popover = new Popover();
240+
popover.setTarget(target);
241+
fakeClientResponse();
242+
243+
target.removeFromParent();
244+
fakeClientResponse();
245+
Assert.assertFalse("Popover should be detached",
246+
popover.getParent().isPresent());
247+
}
248+
249+
@Test
250+
public void targetWithinModalWithSlotDefined_popoverInheritsSlotAttribute() {
251+
var modal = new TestModalContainerWithSlot();
252+
ui.add(modal);
253+
var target = new Div();
254+
var popover = new Popover();
255+
popover.setTarget(target);
256+
modal.add(target);
257+
fakeClientResponse();
258+
259+
Assert.assertEquals("custom-slot",
260+
popover.getElement().getAttribute("slot"));
261+
262+
fakeClientResponse();
263+
var newModal = new TestModalContainer();
264+
ui.add(newModal);
265+
newModal.add(target);
266+
fakeClientResponse();
267+
Assert.assertFalse("Popover should not have value for slot attribute",
268+
popover.getElement().hasAttribute("slot"));
269+
}
270+
271+
@Test
272+
public void popoverWithTargetAddedAsVirtualChild_popoverAttachedToModal() {
273+
var modal = new TestModalComponent();
274+
ui.add(modal);
275+
var target = new Div();
276+
modal.getElement().appendVirtualChild(target.getElement());
277+
var popover = new Popover();
278+
popover.setTarget(target);
279+
fakeClientResponse();
280+
Assert.assertEquals("Popover should be attached to modal", modal,
281+
popover.getParent().orElse(null));
282+
}
283+
187284
private void fakeClientResponse() {
188285
ui.getInternals().getStateTree().runExecutionsBeforeClientResponse();
189286
ui.getInternals().getStateTree().collectChanges(ignore -> {
190287
});
191288
}
289+
290+
@ModalRoot
291+
@Tag("div")
292+
public class TestModalContainer extends Component implements HasComponents {
293+
public TestModalContainer() {
294+
super();
295+
}
296+
}
297+
298+
@ModalRoot
299+
@Tag("div")
300+
public class TestModalComponent extends Component {
301+
public TestModalComponent() {
302+
super();
303+
}
304+
}
305+
306+
@ModalRoot(slot = "custom-slot")
307+
@Tag("div")
308+
public class TestModalContainerWithSlot extends Component
309+
implements HasComponents {
310+
public TestModalContainerWithSlot() {
311+
super();
312+
}
313+
}
192314
}

0 commit comments

Comments
 (0)