diff options
-rw-r--r-- | client/src/main/java/com/vaadin/client/ui/VComboBox.java | 69 | ||||
-rw-r--r-- | uitest/src/main/java/com/vaadin/tests/components/combobox/ComboBoxWithinHorizontalLayout.java | 77 |
2 files changed, 138 insertions, 8 deletions
diff --git a/client/src/main/java/com/vaadin/client/ui/VComboBox.java b/client/src/main/java/com/vaadin/client/ui/VComboBox.java index 438234d30b..c7fe9e14be 100644 --- a/client/src/main/java/com/vaadin/client/ui/VComboBox.java +++ b/client/src/main/java/com/vaadin/client/ui/VComboBox.java @@ -81,6 +81,7 @@ import com.vaadin.client.ui.aria.HandlesAriaRequired; import com.vaadin.client.ui.combobox.ComboBoxConnector; import com.vaadin.client.ui.menubar.MenuBar; import com.vaadin.client.ui.menubar.MenuItem; +import com.vaadin.client.ui.orderedlayout.Slot; import com.vaadin.shared.AbstractComponentState; import com.vaadin.shared.ui.ComponentStateUtil; import com.vaadin.shared.util.SharedUtil; @@ -899,19 +900,27 @@ public class VComboBox extends Composite implements Field, KeyDownHandler, } } - if (offsetWidth + menuMarginBorderPaddingWidth - + left < VComboBox.this.getAbsoluteLeft() - + VComboBox.this.getOffsetWidth()) { - // Popup doesn't reach all the way to the end of the input - // field, filtering may have changed the popup width. - left = VComboBox.this.getAbsoluteLeft(); + int comboBoxLeft = VComboBox.this.getAbsoluteLeft(); + int comboBoxWidth = VComboBox.this.getOffsetWidth(); + if (hasParentWithUnadjustedPositioning()) { + // ComboBox itself may be incorrectly positioned, don't adjust + // popup position yet. Width calculations must be performed + // anyway to avoid flickering. + return; + } + if (left > comboBoxLeft + || offsetWidth + menuMarginBorderPaddingWidth + + left < comboBoxLeft + comboBoxWidth) { + // Popup is positioned too far right or doesn't reach all the + // way to the end of the input field, filtering may have changed + // the popup width. + left = comboBoxLeft; } if (offsetWidth + menuMarginBorderPaddingWidth + left > Window .getClientWidth()) { // Popup doesn't fit the view, needs to be opened to the left // instead. - left = VComboBox.this.getAbsoluteLeft() - + VComboBox.this.getOffsetWidth() - offsetWidth + left = comboBoxLeft + comboBoxWidth - offsetWidth - (int) menuMarginBorderPaddingWidth; } if (left < 0) { @@ -924,6 +933,50 @@ public class VComboBox extends Composite implements Field, KeyDownHandler, } /** + * Checks whether there are any {@link VHorizontalLayout}s with + * incomplete internal position calculations among this VComboBox's + * parents. + * + * @return {@code true} if unadjusted parents found, {@code false} + * otherwise + */ + private boolean hasParentWithUnadjustedPositioning() { + /* + * If there are any VHorizontalLayouts among this VComboBox's + * parents, any spacing or expand ratio may cause incorrect + * intermediate positioning. The status of the layout's internal + * positioning can be checked from the first slot's margin-left + * style, which will be set to 0px if no spacing or expand ratio + * adjustments are needed, and to a negative pixel amount if they + * are. If the style hasn't been set at all, calculations are still + * underway. Popup position shouldn't be adjusted before such + * calculations have been finished. + * + * VVerticalLayout has the same logic but it only affects the + * vertical positioning, which is irrelevant for the calculations + * here. + */ + Widget toCheck = VComboBox.this; + while (toCheck != null && !(toCheck.getParent() instanceof VUI)) { + toCheck = toCheck.getParent(); + if (toCheck instanceof VHorizontalLayout) { + VHorizontalLayout hLayout = (VHorizontalLayout) toCheck; + // because hLayout is a parent it must have at least one + // child widget + Widget slot = hLayout.getWidget(0); + if (slot instanceof Slot && slot.getElement().getStyle() + .getMarginLeft().isEmpty()) { + // margin hasn't been set, layout's internal positioning + // is still being adjusted and ComboBox's position may + // not be final + return true; + } + } + } + return false; + } + + /** * Adds in-line CSS rules to the DOM according to the * suggestionPopupWidth field * diff --git a/uitest/src/main/java/com/vaadin/tests/components/combobox/ComboBoxWithinHorizontalLayout.java b/uitest/src/main/java/com/vaadin/tests/components/combobox/ComboBoxWithinHorizontalLayout.java new file mode 100644 index 0000000000..4ae8977a8b --- /dev/null +++ b/uitest/src/main/java/com/vaadin/tests/components/combobox/ComboBoxWithinHorizontalLayout.java @@ -0,0 +1,77 @@ +package com.vaadin.tests.components.combobox; + +import java.util.Arrays; +import java.util.Iterator; + +import com.vaadin.server.VaadinRequest; +import com.vaadin.tests.components.AbstractTestUI; +import com.vaadin.ui.Alignment; +import com.vaadin.ui.Button; +import com.vaadin.ui.ComboBox; +import com.vaadin.ui.Component; +import com.vaadin.ui.HorizontalLayout; + +public class ComboBoxWithinHorizontalLayout extends AbstractTestUI { + + private ComboBox<String> comboBox; + + @Override + protected void setup(VaadinRequest request) { + comboBox = new ComboBox<>(); + comboBox.setWidth("100%"); + comboBox.setPopupWidth(null); + populateWithShortItems(); + + HorizontalLayout content = new HorizontalLayout(new Button("1"), + new Button("2"), comboBox, new Button("3")); + content.setWidth("100%"); + content.setExpandRatio(comboBox, 2.0f); + Iterator<Component> i = content.iterator(); + while (i.hasNext()) { + content.setComponentAlignment(i.next(), Alignment.BOTTOM_RIGHT); + } + + getLayout().setSpacing(true); + addComponent(content); + addComponent(new Button("Toggle items", e -> { + if ("Short items".equals(comboBox.getCaption())) { + populateWithLongItems(); + } else { + populateWithShortItems(); + } + })); + addComponent(new Button("Toggle spacing", e -> { + content.setSpacing(!content.isSpacing()); + })); + addComponent(new Button("Toggle expand ratio", e -> { + if (content.getExpandRatio(comboBox) > 0.0f) { + content.setExpandRatio(comboBox, 0.0f); + } else { + content.setExpandRatio(comboBox, 2.0f); + } + })); + } + + private void populateWithLongItems() { + comboBox.setCaption("Long items"); + comboBox.setItems(Arrays.asList( + "First very, very, very, very, very long item to add", + "Second very long item to add", "Third very long item to add")); + } + + private void populateWithShortItems() { + comboBox.setCaption("Short items"); + comboBox.setItems(Arrays.asList("short1", "short2", "short3")); + } + + @Override + protected Integer getTicketNumber() { + return 11718; + } + + @Override + protected String getTestDescription() { + return "ComboBox within HorizontalLayout should not get incorrect " + + "intermediate popup positions that cause flickering."; + } +} |