From 46da9629b108fa7977061ac72b9a7a1d1fe80c3d Mon Sep 17 00:00:00 2001 From: Teemu Suo-Anttila Date: Fri, 29 Dec 2017 13:11:20 +0200 Subject: [PATCH] Implement widget event handling for Columns in Grid (#10412) Fixes #7833 --- .../connectors/grid/ColumnConnector.java | 5 ++ .../java/com/vaadin/client/widgets/Grid.java | 63 +++++++++++++++++-- server/src/main/java/com/vaadin/ui/Grid.java | 36 ++++++++++- .../vaadin/shared/ui/grid/ColumnState.java | 2 +- .../tests/components/grid/GridComponents.java | 4 +- .../components/grid/GridComponentsTest.java | 14 +++++ 6 files changed, 114 insertions(+), 10 deletions(-) diff --git a/client/src/main/java/com/vaadin/client/connectors/grid/ColumnConnector.java b/client/src/main/java/com/vaadin/client/connectors/grid/ColumnConnector.java index 1737f90210..20b624892c 100644 --- a/client/src/main/java/com/vaadin/client/connectors/grid/ColumnConnector.java +++ b/client/src/main/java/com/vaadin/client/connectors/grid/ColumnConnector.java @@ -195,6 +195,11 @@ public class ColumnConnector extends AbstractExtensionConnector { column.setTooltipContentMode(getState().tooltipContentMode); } + @OnStateChange("widgetEventsAllowed") + void updateWidgetEventsAllowed() { + column.setWidgetEventsAllowed(getState().widgetEventsAllowed); + } + @Override public void onUnregister() { super.onUnregister(); diff --git a/client/src/main/java/com/vaadin/client/widgets/Grid.java b/client/src/main/java/com/vaadin/client/widgets/Grid.java index d8f0e12b76..405b6ab208 100755 --- a/client/src/main/java/com/vaadin/client/widgets/Grid.java +++ b/client/src/main/java/com/vaadin/client/widgets/Grid.java @@ -2323,9 +2323,7 @@ public class Grid extends ResizeComposite implements HasSelectionHandlers, protected void dispatch(HANDLER handler) { EventTarget target = getNativeEvent().getEventTarget(); Grid grid = getGrid(); - if (Element.is(target) && grid != null - && !grid.isElementInChildWidget(Element.as(target))) { - + if (Element.is(target) && grid != null) { Section section = Section.FOOTER; final RowContainer container = grid.cellFocusHandler.containerWithFocus; if (container == grid.escalator.getHeader()) { @@ -2334,6 +2332,19 @@ public class Grid extends ResizeComposite implements HasSelectionHandlers, section = Section.BODY; } + // Don't handle event of child widget unless the column has been + // explicitly permitted to do so + if (grid.isElementInChildWidget(Element.as(target))) { + Cell cell = container.getCell(target.cast()); + if (cell != null) { + Column column = grid + .getVisibleColumn(cell.getColumn()); + if (column == null || !column.isWidgetEventsAllowed()) { + return; + } + } + } + doDispatch(handler, section); } } @@ -2441,8 +2452,22 @@ public class Grid extends ResizeComposite implements HasSelectionHandlers, */ protected boolean ignoreEventFromTarget(Grid grid, Element targetElement) { - // Target is some widget inside of Grid - return grid.isElementInChildWidget(targetElement); + boolean childWidget = grid.isElementInChildWidget(targetElement); + boolean handleWidgetEvent = false; + + RowContainer container = grid.getEscalator() + .findRowContainer(targetElement); + if (container != null) { + Cell cell = container.getCell(targetElement); + if (cell != null) { + Column column = grid + .getVisibleColumn(cell.getColumn()); + handleWidgetEvent = column != null + && column.isWidgetEventsAllowed(); + } + } + + return childWidget && !handleWidgetEvent; } protected abstract void doDispatch(HANDLER handler, Section section); @@ -4487,7 +4512,9 @@ public class Grid extends ResizeComposite implements HasSelectionHandlers, .toArray(new Column[reordered.size()]); setColumnOrder(true, array); transferCellFocusOnDrop(); - } // else no reordering + } // else + // no + // reordering } private void transferCellFocusOnDrop() { @@ -4735,6 +4762,8 @@ public class Grid extends ResizeComposite implements HasSelectionHandlers, private String hidingToggleCaption = null; + private boolean widgetEventsAllowed = false; + private double minimumWidthPx = GridConstants.DEFAULT_MIN_WIDTH; private double maximumWidthPx = GridConstants.DEFAULT_MAX_WIDTH; private int expandRatio = GridConstants.DEFAULT_EXPAND_RATIO; @@ -5513,6 +5542,28 @@ public class Grid extends ResizeComposite implements HasSelectionHandlers, cell.setText(headerCaption); } + /** + * Returns whether Grid should handle events from Widgets in this + * Column. + * + * @return {@code true} to handle events from widgets; {@code false} to + * not + */ + public boolean isWidgetEventsAllowed() { + return widgetEventsAllowed; + } + + /** + * Sets whether Grid should handle events from Widgets in this Column. + * + * @param widgetEventsAllowed + * {@code true} to let grid handle events from widgets; + * {@code false} to not + */ + public void setWidgetEventsAllowed(boolean widgetEventsAllowed) { + this.widgetEventsAllowed = widgetEventsAllowed; + } + } protected class BodyUpdater implements EscalatorUpdater { diff --git a/server/src/main/java/com/vaadin/ui/Grid.java b/server/src/main/java/com/vaadin/ui/Grid.java index 64a09d8d96..58945bfc46 100644 --- a/server/src/main/java/com/vaadin/ui/Grid.java +++ b/server/src/main/java/com/vaadin/ui/Grid.java @@ -2055,6 +2055,40 @@ public class Grid extends AbstractListing implements HasComponents, return (Renderer) getState().renderer; } + /** + * Sets whether Grid should handle events in this Column from Components + * and Widgets rendered by certain Renderers. By default the events are + * not handled. + *

+ * Note: Enabling this feature will for example select + * a row when a component is clicked. For example in the case of a + * {@link ComboBox} or {@link TextField} it might be problematic as the + * component gets re-rendered and might lose focus. + * + * @param widgetEventsAllowed + * {@code true} to handle events; {@code false} to not + * @return this column + */ + public Column setWidgetEventsAllowed( + boolean widgetEventsAllowed) { + if (getState(false).widgetEventsAllowed != widgetEventsAllowed) { + getState().widgetEventsAllowed = widgetEventsAllowed; + } + return this; + } + + /** + * Gets whether Grid is handling the events in this Column from + * Component and Widgets. + * + * @see #setWidgetEventsAllowed(boolean) + * + * @return {@code true} if handling events; {@code false} if not + */ + public boolean isWidgetEventsAllowed() { + return getState(false).widgetEventsAllowed; + } + /** * Gets the grid that this column belongs to. * @@ -2589,8 +2623,8 @@ public class Grid extends AbstractListing implements HasComponents, *

* You can add columns for nested properties with dot notation, eg. * "property.nestedProperty" - * + * * @param propertyName * the property name of the new column, not null * @param renderer diff --git a/shared/src/main/java/com/vaadin/shared/ui/grid/ColumnState.java b/shared/src/main/java/com/vaadin/shared/ui/grid/ColumnState.java index 652db0c1bf..3b4f2af681 100644 --- a/shared/src/main/java/com/vaadin/shared/ui/grid/ColumnState.java +++ b/shared/src/main/java/com/vaadin/shared/ui/grid/ColumnState.java @@ -30,6 +30,7 @@ public class ColumnState extends AbstractGridExtensionState { public String internalId; public boolean sortable = false; public boolean editable = false; + public boolean widgetEventsAllowed = false; /** The assistive device caption for the column. */ public String assistiveCaption; @@ -81,5 +82,4 @@ public class ColumnState extends AbstractGridExtensionState { * @since 8.2 */ public ContentMode tooltipContentMode; - } diff --git a/uitest/src/main/java/com/vaadin/tests/components/grid/GridComponents.java b/uitest/src/main/java/com/vaadin/tests/components/grid/GridComponents.java index 22e880573c..df71987955 100644 --- a/uitest/src/main/java/com/vaadin/tests/components/grid/GridComponents.java +++ b/uitest/src/main/java/com/vaadin/tests/components/grid/GridComponents.java @@ -26,8 +26,8 @@ public class GridComponents extends AbstractTestUIWithLog { @Override protected void setup(VaadinRequest request) { Grid grid = new Grid<>(); - grid.addColumn(string -> new Label(string), new ComponentRenderer()) - .setId("label").setCaption("Column 0"); + grid.addComponentColumn(Label::new).setId("label") + .setCaption("Column 0").setWidgetEventsAllowed(true); grid.getDefaultHeaderRow().getCell("label") .setComponent(new Label("Label")); grid.addComponentColumn(string -> { diff --git a/uitest/src/test/java/com/vaadin/tests/components/grid/GridComponentsTest.java b/uitest/src/test/java/com/vaadin/tests/components/grid/GridComponentsTest.java index 4a303ddfb5..b9cc1fef35 100644 --- a/uitest/src/test/java/com/vaadin/tests/components/grid/GridComponentsTest.java +++ b/uitest/src/test/java/com/vaadin/tests/components/grid/GridComponentsTest.java @@ -9,6 +9,7 @@ import java.util.stream.IntStream; import java.util.stream.Stream; import org.junit.Test; +import org.openqa.selenium.Keys; import org.openqa.selenium.WebElement; import com.vaadin.testbench.By; @@ -139,6 +140,19 @@ public class GridComponentsTest extends MultiBrowserTest { assertEquals("Other Components", grid.getHeaderCell(0, 1).getText()); } + @Test + public void testSelectRowByClickingLabel() { + openTestURL(); + + GridElement grid = $(GridElement.class).first(); + assertFalse("Row should not be initially selected", + grid.getRow(0).isSelected()); + + grid.getCell(0, 0).$(LabelElement.class).first().click(10, 10, + new Keys[0]); + assertTrue("Row should be selected", grid.getRow(0).isSelected()); + } + private void assertRowExists(int i, String string) { GridRowElement row = $(GridElement.class).first().getRow(i); assertEquals("Label text did not match", string, -- 2.39.5