From bad0f9a26b856b7891c26e5a94b1f3ad2fafbac1 Mon Sep 17 00:00:00 2001 From: Artur Signell Date: Fri, 25 Nov 2011 11:53:09 +0000 Subject: [PATCH] Merged fixes from 6.7 svn changeset:22132/svn branch:6.8 --- .../vaadin/data/util/PropertyFormatter.java | 20 +++-- .../terminal/gwt/client/ui/VScrollTable.java | 8 ++ .../terminal/gwt/client/ui/VWindow.java | 48 ++++++---- src/com/vaadin/ui/Form.java | 30 ++++++- src/com/vaadin/ui/Table.java | 46 +++++++--- .../tests/server/TestPropertyFormatter.java | 69 ++++++++++++++ .../FormWithPropertyFormatterConnected.java | 90 +++++++++++++++++++ .../window/SubWindowWithUndefinedHeight.html | 52 +++++++++++ .../window/SubWindowWithUndefinedHeight.java | 64 +++++++++++++ 9 files changed, 387 insertions(+), 40 deletions(-) create mode 100644 tests/server-side/com/vaadin/tests/server/TestPropertyFormatter.java create mode 100644 tests/testbench/com/vaadin/tests/components/form/FormWithPropertyFormatterConnected.java create mode 100644 tests/testbench/com/vaadin/tests/components/window/SubWindowWithUndefinedHeight.html create mode 100644 tests/testbench/com/vaadin/tests/components/window/SubWindowWithUndefinedHeight.java diff --git a/src/com/vaadin/data/util/PropertyFormatter.java b/src/com/vaadin/data/util/PropertyFormatter.java index c43a4771dc..ae9b0d40d1 100644 --- a/src/com/vaadin/data/util/PropertyFormatter.java +++ b/src/com/vaadin/data/util/PropertyFormatter.java @@ -34,7 +34,8 @@ import com.vaadin.data.Property; */ @SuppressWarnings("serial") public abstract class PropertyFormatter extends AbstractProperty implements - Property.ValueChangeListener, Property.ReadOnlyStatusChangeListener { + Property.Viewer, Property.ValueChangeListener, + Property.ReadOnlyStatusChangeListener { /** Datasource that stores the actual value. */ Property dataSource; @@ -146,7 +147,10 @@ public abstract class PropertyFormatter extends AbstractProperty implements */ @Override public String toString() { - Object value = dataSource == null ? false : dataSource.getValue(); + if (dataSource == null) { + return null; + } + Object value = dataSource.getValue(); if (value == null) { return null; } @@ -154,6 +158,7 @@ public abstract class PropertyFormatter extends AbstractProperty implements } /** Reflects the read-only status of the datasource. */ + @Override public boolean isReadOnly() { return dataSource == null ? false : dataSource.isReadOnly(); } @@ -190,6 +195,7 @@ public abstract class PropertyFormatter extends AbstractProperty implements * @param newStatus * the new read-only status of the Property. */ + @Override public void setReadOnly(boolean newStatus) { if (dataSource != null) { dataSource.setReadOnly(newStatus); @@ -208,16 +214,14 @@ public abstract class PropertyFormatter extends AbstractProperty implements } } else { try { - dataSource.setValue(parse((String) newValue)); + dataSource.setValue(parse(newValue.toString())); if (!newValue.equals(toString())) { fireValueChange(); } + } catch (ConversionException e) { + throw e; } catch (Exception e) { - if (e instanceof ConversionException) { - throw (ConversionException) e; - } else { - throw new ConversionException(e); - } + throw new ConversionException(e); } } } diff --git a/src/com/vaadin/terminal/gwt/client/ui/VScrollTable.java b/src/com/vaadin/terminal/gwt/client/ui/VScrollTable.java index 959b92cffa..c919d82923 100644 --- a/src/com/vaadin/terminal/gwt/client/ui/VScrollTable.java +++ b/src/com/vaadin/terminal/gwt/client/ui/VScrollTable.java @@ -5704,6 +5704,14 @@ public class VScrollTable extends FlowPanel implements Table, ScrollHandler, if (this.width.equals(width)) { return; } + if (!isVisible()) { + /* + * Do not update size when the table is hidden as all column widths + * will be set to zero and they won't be recalculated when the table + * is set visible again (until the size changes again) + */ + return; + } this.width = width; if (width != null && !"".equals(width)) { diff --git a/src/com/vaadin/terminal/gwt/client/ui/VWindow.java b/src/com/vaadin/terminal/gwt/client/ui/VWindow.java index 8ffb0246a3..8c9fff889d 100644 --- a/src/com/vaadin/terminal/gwt/client/ui/VWindow.java +++ b/src/com/vaadin/terminal/gwt/client/ui/VWindow.java @@ -1027,7 +1027,7 @@ public class VWindow extends VOverlay implements Container, } @Override - /* + /** * Width is set to the out-most element (v-window). * * This function should never be called with percentage values (it will @@ -1088,33 +1088,45 @@ public class VWindow extends VOverlay implements Container, } @Override - /* + /** * Height is set to the out-most element (v-window). * * This function should never be called with percentage values (it will * throw an exception) + * + * @param height A CSS string specifying the new height of the window. + * An empty string or null clears the height and lets + * the browser to compute it based on the window contents. */ public void setHeight(String height) { - this.height = height; - if (!isAttached()) { + if (!isAttached() || (height == null + ? this.height == null + : height.equals(this.height))) { return; } - if (height != null && !"".equals(height)) { - DOM.setStyleAttribute(getElement(), "height", height); - int pixels = getElement().getOffsetHeight() - getExtraHeight(); - if (pixels < MIN_CONTENT_AREA_HEIGHT) { - pixels = MIN_CONTENT_AREA_HEIGHT; - int rootHeight = pixels + getExtraHeight(); - DOM.setStyleAttribute(getElement(), "height", (rootHeight) - + "px"); - + if (height == null || "".equals(height)) { + getElement().getStyle().clearHeight(); + contentPanel.getElement().getStyle().clearHeight(); + // Reset to default, the exact value does not actually + // matter as an undefined-height parent should not have + // a relative-height child anyway. + renderSpace.setHeight(MIN_CONTENT_AREA_HEIGHT); + } else { + getElement().getStyle().setProperty("height", height); + int contentHeight = + getElement().getOffsetHeight() - getExtraHeight(); + if (contentHeight < MIN_CONTENT_AREA_HEIGHT) { + contentHeight = MIN_CONTENT_AREA_HEIGHT; + int rootHeight = contentHeight + getExtraHeight(); + getElement().getStyle().setProperty( + "height", rootHeight + "px"); } - renderSpace.setHeight(pixels); - height = pixels + "px"; - contentPanel.getElement().getStyle().setProperty("height", height); - updateShadowSizeAndPosition(); - + renderSpace.setHeight(contentHeight); + contentPanel.getElement().getStyle().setProperty( + "height", contentHeight + "px"); } + this.height = height; + updateShadowSizeAndPosition(); } private int extraH = 0; diff --git a/src/com/vaadin/ui/Form.java b/src/com/vaadin/ui/Form.java index 8ee702bbb4..1a28c679f9 100644 --- a/src/com/vaadin/ui/Form.java +++ b/src/com/vaadin/ui/Form.java @@ -491,7 +491,7 @@ public class Form extends AbstractField implements Item.Editor, Buffered, Item, } // Configures the field - field.setPropertyDataSource(property); + bindPropertyToField(id, property, field); // Register and attach the created field addField(id, field); @@ -755,13 +755,39 @@ public class Form extends AbstractField implements Item.Editor, Buffered, Item, final Field f = fieldFactory.createField(itemDatasource, id, this); if (f != null) { - f.setPropertyDataSource(property); + bindPropertyToField(id, property, f); addField(id, f); } } } } + /** + * Binds an item property to a field. The default behavior is to bind + * property straight to Field. If Property.Viewer type property (e.g. + * PropertyFormatter) is already set for field, the property is bound to + * that Property.Viewer. + * + * @param propertyId + * @param property + * @param field + * @since 6.7.3 + */ + protected void bindPropertyToField(final Object propertyId, + final Property property, final Field field) { + // check if field has a property that is Viewer set. In that case we + // expect developer has e.g. PropertyFormatter that he wishes to use and + // assign the property to the Viewer instead. + boolean hasFilterProperty = field.getPropertyDataSource() != null + && (field.getPropertyDataSource() instanceof Property.Viewer); + if (hasFilterProperty) { + ((Property.Viewer) field.getPropertyDataSource()) + .setPropertyDataSource(property); + } else { + field.setPropertyDataSource(property); + } + } + /** * Gets the layout of the form. * diff --git a/src/com/vaadin/ui/Table.java b/src/com/vaadin/ui/Table.java index 5efb2545e0..b95d738157 100644 --- a/src/com/vaadin/ui/Table.java +++ b/src/com/vaadin/ui/Table.java @@ -3405,7 +3405,7 @@ public class Table extends AbstractSelect implements Action.Container, // Remember that we have made this association so we can remove // it when the component is removed associatedProperties.put(f, property); - f.setPropertyDataSource(property); + bindPropertyToField(rowId, colId, property, f); return f; } } @@ -3413,6 +3413,33 @@ public class Table extends AbstractSelect implements Action.Container, return formatPropertyValue(rowId, colId, property); } + /** + * Binds an item property to a field generated by TableFieldFactory. The + * default behavior is to bind property straight to Field. If + * Property.Viewer type property (e.g. PropertyFormatter) is already set for + * field, the property is bound to that Property.Viewer. + * + * @param rowId + * @param colId + * @param property + * @param field + * @since 6.7.3 + */ + protected void bindPropertyToField(Object rowId, Object colId, + Property property, Field field) { + // check if field has a property that is Viewer set. In that case we + // expect developer has e.g. PropertyFormatter that he wishes to use and + // assign the property to the Viewer instead. + boolean hasFilterProperty = field.getPropertyDataSource() != null + && (field.getPropertyDataSource() instanceof Property.Viewer); + if (hasFilterProperty) { + ((Property.Viewer) field.getPropertyDataSource()) + .setPropertyDataSource(property); + } else { + field.setPropertyDataSource(property); + } + } + /** * Formats table cell property values. By default the property.toString() * and return a empty string for null properties. @@ -3775,8 +3802,8 @@ public class Table extends AbstractSelect implements Action.Container, *

* Note, that some due to historical reasons the name of the method is bit * misleading. Some items may be partly or totally out of the viewport of - * the table's scrollable area. Actully detecting rows which can be actually - * seen by the end user may be problematic due to the client server + * the table's scrollable area. Actually detecting rows which can be + * actually seen by the end user may be problematic due to the client server * architecture. Using {@link #getCurrentPageFirstItemId()} combined with * {@link #getPageLength()} may produce good enough estimates in some * situations. @@ -4507,19 +4534,14 @@ public class Table extends AbstractSelect implements Action.Container, * com.vaadin.event.dd.acceptcriteria.AcceptCriterion#accepts(com.vaadin * .event.dd.DragAndDropEvent) */ + @SuppressWarnings("unchecked") public boolean accept(DragAndDropEvent dragEvent) { AbstractSelectTargetDetails dropTargetData = (AbstractSelectTargetDetails) dragEvent .getTargetDetails(); table = (Table) dragEvent.getTargetDetails().getTarget(); - ArrayList visibleItemIds = new ArrayList( - table.getPageLength()); - visibleItemIds.size(); - Object id = table.getCurrentPageFirstItemId(); - for (int i = 0; i < table.getPageLength() && id != null; i++) { - visibleItemIds.add(id); - id = table.nextItemId(id); - } - allowedItemIds = getAllowedItemIds(dragEvent, table, visibleItemIds); + Collection visibleItemIds = table.getVisibleItemIds(); + allowedItemIds = getAllowedItemIds(dragEvent, table, + (Collection) visibleItemIds); return allowedItemIds.contains(dropTargetData.getItemIdOver()); } diff --git a/tests/server-side/com/vaadin/tests/server/TestPropertyFormatter.java b/tests/server-side/com/vaadin/tests/server/TestPropertyFormatter.java new file mode 100644 index 0000000000..91e36b5caa --- /dev/null +++ b/tests/server-side/com/vaadin/tests/server/TestPropertyFormatter.java @@ -0,0 +1,69 @@ +package com.vaadin.tests.server; + +import java.util.Date; + +import junit.framework.TestCase; + +import org.junit.Test; + +import com.vaadin.data.util.ObjectProperty; +import com.vaadin.data.util.PropertyFormatter; + +@SuppressWarnings("unchecked") +public class TestPropertyFormatter extends TestCase { + + class TestFormatter extends PropertyFormatter { + + @Override + public String format(Object value) { + boolean isCorrectType = getExpectedClass().isAssignableFrom( + value.getClass()); + assertTrue(isCorrectType); + return "FOO"; + } + + @Override + public Object parse(String formattedValue) throws Exception { + return getExpectedClass().newInstance(); + } + }; + @SuppressWarnings("rawtypes") + private Class expectedClass; + + @SuppressWarnings("rawtypes") + private Class getExpectedClass() { + return expectedClass; + } + + /** + * The object passed to format should be same as property's type. + * @throws IllegalAccessException + * @throws InstantiationException + */ + @Test + @SuppressWarnings({ "rawtypes" }) + public void testCorrectTypeForFormat() throws InstantiationException, IllegalAccessException { + Class[] testedTypes = new Class[] {Integer.class, Boolean.class, Double.class, String.class, Date.class}; + Object[] testValues = new Object[] {new Integer(3), Boolean.FALSE, new Double(3.3), "bar", new Date()}; + + int i = 0; + for (Class class1 : testedTypes) { + expectedClass = class1; + + TestFormatter formatter = new TestFormatter(); + + // Should just return null, without formatting + Object value = formatter.getValue(); + + // test with property which value is null + formatter.setPropertyDataSource(new ObjectProperty(null, expectedClass)); + formatter.getValue(); // calls format + + // test with a value + formatter.setPropertyDataSource(new ObjectProperty(testValues[i++], expectedClass)); + formatter.getValue(); // calls format + } + + + } +} diff --git a/tests/testbench/com/vaadin/tests/components/form/FormWithPropertyFormatterConnected.java b/tests/testbench/com/vaadin/tests/components/form/FormWithPropertyFormatterConnected.java new file mode 100644 index 0000000000..116dd5bcc7 --- /dev/null +++ b/tests/testbench/com/vaadin/tests/components/form/FormWithPropertyFormatterConnected.java @@ -0,0 +1,90 @@ +package com.vaadin.tests.components.form; + +import com.vaadin.data.Item; +import com.vaadin.data.util.BeanItem; +import com.vaadin.data.util.PropertyFormatter; +import com.vaadin.tests.components.TestBase; +import com.vaadin.ui.AbstractField; +import com.vaadin.ui.Button; +import com.vaadin.ui.Component; +import com.vaadin.ui.DefaultFieldFactory; +import com.vaadin.ui.Field; +import com.vaadin.ui.Form; +import com.vaadin.ui.FormFieldFactory; + +public class FormWithPropertyFormatterConnected extends TestBase { + @Override + protected void setup() { + Form form2 = new Form(); + form2.setFormFieldFactory(new FormFieldFactory() { + + public Field createField(Item item, Object propertyId, + Component uiContext) { + AbstractField f = (AbstractField) DefaultFieldFactory.get() + .createField(item, propertyId, uiContext); + if (propertyId.equals("age")) { + f.setPropertyDataSource(new PropertyFormatter() { + + @Override + public Object parse(String formattedValue) + throws Exception { + String str = formattedValue + .replaceAll("[^0-9.]", ""); + if (formattedValue.toLowerCase().contains("months")) { + return Double.parseDouble(str) / 12; + } + return Double.parseDouble(str); + } + + @Override + public String format(Object value) { + Double dValue = (Double) value; + if (dValue < 1) { + return ((int)(dValue * 12)) + " months"; + } + return dValue + " years"; + } + }); + f.setImmediate(true); + } + return f; + } + }); + form2.setItemDataSource(createItem()); + + addComponent(form2); + addComponent(new Button("B")); + } + + private Item createItem() { + return new BeanItem(new Person(0.5)); + } + + public class Person { + public Person(double age) { + super(); + this.age = age; + } + + public double getAge() { + return age; + } + + public void setAge(double age) { + this.age = age; + } + + private double age; + } + + @Override + protected String getDescription() { + return "It should be possible to inject PropertyFormatter and similar classses to fields in form. The test app hooks formatter that displays age in years or months and also accepts value in both (years by default, months if mentioned in the field)"; + } + + @Override + protected Integer getTicketNumber() { + return null; + } + +} diff --git a/tests/testbench/com/vaadin/tests/components/window/SubWindowWithUndefinedHeight.html b/tests/testbench/com/vaadin/tests/components/window/SubWindowWithUndefinedHeight.html new file mode 100644 index 0000000000..2c3f651e41 --- /dev/null +++ b/tests/testbench/com/vaadin/tests/components/window/SubWindowWithUndefinedHeight.html @@ -0,0 +1,52 @@ + + + + + + +SubWindowWithUndefinedHeight + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
SubWindowWithUndefinedHeight
open/run/com.vaadin.tests.components.window.SubWindowWithUndefinedHeight?restartApplication
clickvaadin=runcomvaadintestscomponentswindowSubWindowWithUndefinedHeight::/VVerticalLayout[0]/ChildComponentContainer[2]/VButton[0]/domChild[0]
screenCaptureinitial-tab1
mouseClickvaadin=runcomvaadintestscomponentswindowSubWindowWithUndefinedHeight::/VWindow[0]/FocusableScrollPanel[0]/VVerticalLayout[0]/ChildComponentContainer[0]/VTabsheet[0]/domChild[0]/domChild[0]/domChild[0]/domChild[0]/domChild[1]/domChild[0]/domChild[0]/domChild[0]17,13
screenCaptureselect-tab2
mouseClickvaadin=runcomvaadintestscomponentswindowSubWindowWithUndefinedHeight::/VWindow[0]/FocusableScrollPanel[0]/VVerticalLayout[0]/ChildComponentContainer[0]/VTabsheet[0]/domChild[0]/domChild[0]/domChild[0]/domChild[0]/domChild[0]/domChild[0]/domChild[0]/domChild[0]8,9
screenCaptureselect-tab1
+ + diff --git a/tests/testbench/com/vaadin/tests/components/window/SubWindowWithUndefinedHeight.java b/tests/testbench/com/vaadin/tests/components/window/SubWindowWithUndefinedHeight.java new file mode 100644 index 0000000000..dbe5eda9af --- /dev/null +++ b/tests/testbench/com/vaadin/tests/components/window/SubWindowWithUndefinedHeight.java @@ -0,0 +1,64 @@ +package com.vaadin.tests.components.window; + +import com.vaadin.tests.components.TestBase; +import com.vaadin.ui.Window; +import com.vaadin.ui.Button; +import com.vaadin.ui.Table; +import com.vaadin.ui.TabSheet; + +public class SubWindowWithUndefinedHeight extends TestBase { + + @Override + protected String getDescription() { + return "Setting subwindow height to undefined after initial rendering does not update visual height"; + } + + @Override + protected Integer getTicketNumber() { + return 7916; + } + + @Override + protected void setup() { + final Window subwindow = new Window("subwindow"); + subwindow.center(); + subwindow.setSizeUndefined(); + subwindow.getContent().setSizeUndefined(); + + final Button tabButton = new Button("A button"); + tabButton.setCaption("Tab 1"); + tabButton.setWidth("200px"); + + final Table table = new Table(); + table.setCaption("tab 2"); + table.setWidth("100%"); + table.setHeight("100%"); + + final TabSheet tabsheet = new TabSheet(); + tabsheet.addComponent(tabButton); + tabsheet.addComponent(table); + tabsheet.addListener(new TabSheet.SelectedTabChangeListener() { + public void selectedTabChange( + TabSheet.SelectedTabChangeEvent event) { + if (tabsheet.getSelectedTab() == tabButton) { + tabsheet.setSizeUndefined(); + subwindow.getContent().setSizeUndefined(); + subwindow.setSizeUndefined(); + } else if (tabsheet.getSelectedTab() == table) { + subwindow.setWidth("500px"); + subwindow.setHeight("500px"); + subwindow.getContent().setSizeFull(); + tabsheet.setSizeFull(); + } + } + }); + subwindow.addComponent(tabsheet); + + Button button = new Button("click me", new Button.ClickListener() { + public void buttonClick(Button.ClickEvent event) { + getMainWindow().addWindow(subwindow); + } + }); + getMainWindow().addComponent(button); + } +} -- 2.39.5