PDA

View Full Version : [FIXED] odd inline editor error with Grid



nbuesing
6 Jan 2012, 8:34 AM
I want to report this, wondering/hoping if someone has ideas (or can confirm this is a defect). Otherwise, I will try to add more context as I figure it out.

Environment Information: I am using GXT 3.0.0-beta2; Internet Explorer 8 & latest version of Chrome on Windows 7 64bit machine.

I have a grid with Numeric Fields. Sometimes when I enter a single value in a field and then navigate away (via tab or click), the cell's value isn't updated (cell remains blank) and the modified symbol (red triangle) is not displayed. Then if I go back to the same cell or a different cell (same column, but the other cell is currently blank) the editor has the value that was entered int he previous cell.

Now, it seems like the smaller the number, the greater chance of this happening. Once I get the cell to take value, then it seems to work just fine. It seems like the editor's value is not applied to the cell upon the dismissing the editor (and the editor continues to remember the value that it was editing -- which seems to indicate the editor's value is never pushed to the cell when "onBlur" occurs.

For the editor I'm using a NumberField with BigDecimalPropertyEditor I created (extending NumberPropertyEditor<BigDecimal>).

Colin Alworth
6 Jan 2012, 9:58 AM
Thanks for the report - does this happen during both dev mode and prod mode? Also, can you build a simple entrypoint that builds the grid/editor and BigDecimalPropertyEditor you are using so we can try to reproduce the exact issue you are seeing?

nbuesing
6 Jan 2012, 10:15 AM
Yes this does happen in both dev mode and prod mode. yes, I will look into trying to duplicate and provide an example.

nbuesing
10 Jan 2012, 10:42 AM
I was able to reproduce the problem with your "Inline Editable Grid" example.

Here are my changes

o Change Plant Price from double to BigDecimal.
o Create a BigDecimalPropertyEditor.
o Changed price to just return a numeric value (no $) and provided an editor for the cell.

It isn't always reproducable, but the easiest way to reproduce.

Hightlight a cell, and add a digit to the beginning of the cell, select 2.44 cell and add a 1 in front of it to make it 12.44. Now if I erase 2.44 and then enter 12.44, then the edit will stick, it is when I just insert a number in front of the number that is already there, that is when the edit doesn't get applied.

The error occurs when I tab navigate or click on another cell.

Thanks.




Index: com.sencha.gxt.examples/src/main/java/com/sencha/gxt/examples/resources/client/model/Plant.java
===================================================================
--- com.sencha.gxt.examples/src/main/java/com/sencha/gxt/examples/resources/client/model/Plant.java (revision 2025)
+++ com.sencha.gxt.examples/src/main/java/com/sencha/gxt/examples/resources/client/model/Plant.java (working copy)
@@ -1,5 +1,6 @@
package com.sencha.gxt.examples.resources.client.model;

+import java.math.BigDecimal;
import java.util.Date;

import com.google.gwt.i18n.client.DateTimeFormat;
@@ -10,7 +11,7 @@

private String name;
private String light;
- private double price;
+ private BigDecimal price;
private Date available;
private boolean indoor;

@@ -19,6 +20,14 @@
}

public Plant(String name, String light, double price, String available, boolean indoor) {
+ setName(name);
+ setLight(light);
+ setPrice(new BigDecimal(Double.valueOf(price).toString()));
+ setAvailable(df.parse(available));
+ setIndoor(indoor);
+ }
+
+ public Plant(String name, String light, BigDecimal price, String available, boolean indoor) {
setName(name);
setLight(light);
setPrice(price);
@@ -38,7 +47,7 @@
return name;
}

- public double getPrice() {
+ public BigDecimal getPrice() {
return price;
}

@@ -62,7 +71,7 @@
this.name = name;
}

- public void setPrice(double price) {
+ public void setPrice(BigDecimal price) {
this.price = price;
}

Index: com.sencha.gxt.examples/src/main/java/com/sencha/gxt/examples/resources/client/model/PlantProxy.java
===================================================================
--- com.sencha.gxt.examples/src/main/java/com/sencha/gxt/examples/resources/client/model/PlantProxy.java (revision 2025)
+++ com.sencha.gxt.examples/src/main/java/com/sencha/gxt/examples/resources/client/model/PlantProxy.java (working copy)
@@ -1,5 +1,6 @@
package com.sencha.gxt.examples.resources.client.model;

+import java.math.BigDecimal;
import java.util.Date;


@@ -21,8 +22,8 @@

public void setName(String name);

- public double getPrice();
+ public BigDecimal getPrice();

- public void setPrice(double price);
+ public void setPrice(BigDecimal price);

}
Index: com.sencha.gxt.examples/src/main/java/com/sencha/gxt/explorer/client/grid/AbstractGridEditingExample.java
===================================================================
--- com.sencha.gxt.examples/src/main/java/com/sencha/gxt/explorer/client/grid/AbstractGridEditingExample.java (revision 2025)
+++ com.sencha.gxt.examples/src/main/java/com/sencha/gxt/explorer/client/grid/AbstractGridEditingExample.java (working copy)
@@ -1,5 +1,6 @@
package com.sencha.gxt.explorer.client.grid;

+import java.math.BigDecimal;
import java.text.ParseException;
import java.util.ArrayList;
import java.util.Date;
@@ -11,7 +12,6 @@
import com.google.gwt.editor.client.Editor.Path;
import com.google.gwt.i18n.client.DateTimeFormat;
import com.google.gwt.i18n.client.DateTimeFormat.PredefinedFormat;
-import com.google.gwt.i18n.client.NumberFormat;
import com.google.gwt.safehtml.shared.SafeHtml;
import com.google.gwt.safehtml.shared.SafeHtmlUtils;
import com.google.gwt.text.shared.AbstractSafeHtmlRenderer;
@@ -40,6 +40,7 @@
import com.sencha.gxt.widget.core.client.form.CheckBox;
import com.sencha.gxt.widget.core.client.form.DateField;
import com.sencha.gxt.widget.core.client.form.DateTimePropertyEditor;
+import com.sencha.gxt.widget.core.client.form.NumberField;
import com.sencha.gxt.widget.core.client.form.PropertyEditor;
import com.sencha.gxt.widget.core.client.form.SimpleComboBox;
import com.sencha.gxt.widget.core.client.form.TextField;
@@ -93,7 +94,7 @@

ValueProvider<Plant, Boolean> indoor();

- ValueProvider<Plant, Double> price();
+ ValueProvider<Plant, BigDecimal> price();
}

private static final PlaceProperties properties = GWT.create(PlaceProperties.class);
@@ -120,12 +121,16 @@
}
}));

- ColumnConfig<Plant, Double> cc5 = new ColumnConfig<Plant, Double>(properties.price(), 100, "Price");
+ ColumnConfig<Plant, BigDecimal> cc5 = new ColumnConfig<Plant, BigDecimal>(properties.price(), 100, "Price");
cc5.setAlignment(HasHorizontalAlignment.ALIGN_RIGHT);
- cc5.setCell(new SimpleSafeHtmlCell<Double>(new AbstractSafeHtmlRenderer<Double>() {
+ cc5.setCell(new SimpleSafeHtmlCell<BigDecimal>(new AbstractSafeHtmlRenderer<BigDecimal>() {
@Override
- public SafeHtml render(Double object) {
- return SafeHtmlUtils.fromString(NumberFormat.getCurrencyFormat().format(object));
+ public SafeHtml render(BigDecimal object) {
+ if (object == null) {
+ return SafeHtmlUtils.fromString("");
+ } else {
+ return SafeHtmlUtils.fromString(object.toString());
+ }
}
}));

@@ -148,6 +153,8 @@
final GridEditing<Plant> editing = createGridEditing(grid);
editing.addEditor(cc1, new TextField());

+ editing.addEditor(cc5, new NumberField<BigDecimal>(new BigDecimalPropertyEditor()));
+
SimpleComboBox<Light> combo = new SimpleComboBox<Light>(new StringLabelProvider<Light>());
combo.setPropertyEditor(new PropertyEditor<Light>() {

@@ -208,7 +215,7 @@
Plant plant = new Plant();
plant.setName("New Plant 1");
plant.setLight("Mostly Shady");
- plant.setPrice(0);
+ plant.setPrice(BigDecimal.ZERO);
plant.setAvailable(new DateWrapper().clearTime().asDate());
plant.setIndoor(false);

Index: com.sencha.gxt.examples/src/main/java/com/sencha/gxt/explorer/client/grid/BigDecimalPropertyEditor.java
===================================================================
--- com.sencha.gxt.examples/src/main/java/com/sencha/gxt/explorer/client/grid/BigDecimalPropertyEditor.java (revision 0)
+++ com.sencha.gxt.examples/src/main/java/com/sencha/gxt/explorer/client/grid/BigDecimalPropertyEditor.java (revision 0)
@@ -0,0 +1,51 @@
+package com.sencha.gxt.explorer.client.grid;
+
+import java.math.BigDecimal;
+
+import com.google.gwt.i18n.client.NumberFormat;
+import com.sencha.gxt.widget.core.client.form.NumberPropertyEditor;
+
+/**
+ * @author nxbuesin
+ *
+ */
+public class BigDecimalPropertyEditor extends NumberPropertyEditor<BigDecimal> {
+
+ public BigDecimalPropertyEditor() {
+ super(new BigDecimal("1"));
+ }
+
+ public BigDecimalPropertyEditor(NumberFormat format) {
+ super(format, new BigDecimal("1"));
+ }
+
+ @Override
+ public BigDecimal doIncr(BigDecimal value) {
+ return value.add(getIncrement());
+ }
+
+ @Override
+ public BigDecimal doDecr(BigDecimal value) {
+ return value.subtract(getIncrement());
+ }
+
+ @Override
+ protected BigDecimal parseString(String string) {
+ BigDecimal result = null;
+ try {
+ result = new BigDecimal(string);
+ } catch (Exception e) {
+ System.out.println(e);
+ }
+ return result;
+ }
+
+ @Override
+ protected BigDecimal returnTypedValue(Number number) {
+ if (number instanceof BigDecimal) {
+ return (BigDecimal) number;
+ } else {
+ return new BigDecimal(number.toString());
+ }
+ }
+}

nbuesing
23 Jan 2012, 9:50 PM
Here are some observations in trying to figure out what is going on here.

* If I change the entire value of the cell, the onValueChange event is triggered. If I insert a number prior to the number already there, then no onValueChanged handler is called, only the onBlur in the BlurHandler is called.


GridInlineEditor




fieldRegistration.add(field.addValueChangeHandler(new ValueChangeHandler<O>() {


@Override
public void onValueChange(ValueChangeEvent<O> event) {
// logger.finest("doStartEditing onValueChanged");
completeEditing();
}
}));


fieldRegistration.add(field.addBlurHandler(new BlurHandler() {


@Override
public void onBlur(final BlurEvent event) {
// logger.finest("doStartEditing onBlur");
cancelEditing();
}
}));
fireEvent(new StartEditEvent<M>(cell));


* Based on the debugger the last value already has the new value in it when this is called, so it doesn't believe anything has been changed so it calls the blur event, instead of the value changed event


TriggerFieldCell.finishEditing()


if (valueUpdater != null && !vd.getCurrentValue().equals(vd.getLastValue())) {


When this is called, the vd.getLastValue returns the value that was just typed in, not the old value.

So, somehow the FieldViewData is getting updated earlier than it should be (I think).

So, tracking things back even father, looks like the "keyup" event is being handled and when handled here, the FieldRecordKey is being created with the current value in the cell.

This is why when I enter a single character and then leave the cell, it is treated as unchanged, but if I enter in two characters, and then leave the cell, things seem to work.

TriggerFieldCell.onBrowserEvent


public void onBrowserEvent(Context context, Element parent, T value, NativeEvent event, ValueUpdater<T> valueUpdater) {
super.onBrowserEvent(context, parent, value, event, valueUpdater);
String eventType = event.getType();
Object key = context.getKey();
if ("click".equals(eventType)) {
onClick(context, parent.<XElement> cast(), event, value, valueUpdater);
} else if ("keyup".equals(eventType)) {
// Record keys as they are typed.
FieldViewData vd = getViewData(key);
if (vd == null) {
vd = new FieldViewData(getText(XElement.as(parent)));
setViewData(key, vd);
}
vd.setCurrentValue(getText(XElement.as(parent)));
}
}


So if cell has 5 in it and I insert a 1 in front of it

the "keup" will create a new FieldViewData() with old value of 15, so if I leave the cell at this time, it views it as unchanged, but if I then add another character (say 2) and get 125, then the old value is still 15, so it is viewed as being changed (since vd != null when the second character is typed).

Now if I change "keyup" to "keydown", the last value gets populated with the cell prior to the value being inserted so the last value is indeed the non-edited value, and it seems to work.

nbuesing
6 Mar 2012, 12:34 PM
This error still exists in beta4

I saw major changes to the TriggerFieldCell with beta4 (so i was hopeful), but issue still occurs.

I have provided a simple fix (switch keyup with keydown), would like to know if this is indeed how the issue will be addressed/fixed.

Thanks.

WesleyMoy
7 Mar 2012, 8:06 PM
Thanks for the detailed analysis and for following up after the release of the latest beta. I have more than enough here to file a bug against the team. I'll update this thread with any progress we make.

darrellmeyer
9 Mar 2012, 2:21 PM
This is now fixed in SVN. Thanks for taking the time to investigate.

The bug was that on key up the view data instance was being created using the current string value from the input field. This is wrong, it should be the last actual value not derived from the input element.


} else if ("keyup".equals(eventType)) {
// Record keys as they are typed.
FieldViewData vd = getViewData(key);
if (vd == null) {
// BUG WAS HERE, fix is using the actual value passed in onBrowserEvent
// Now, last value and current value will not be equal if value has changed
vd = new FieldViewData(value == null ? "" : getPropertyEditor().render(value));
setViewData(key, vd);
}
vd.setCurrentValue(getText(XElement.as(parent)));
}

I have also added BigDecimalPropertyEditor to the library.

WesleyMoy
28 Mar 2012, 4:39 PM
This bug has been fixed in the Ext GWT 3.0 Release Candidate. Please upgrade your copy of Ext GWT and try your test case again. While we're confident that we've addressed this issue, please reply if you notice any continued problems after upgrading. Again, thanks for taking the time to report this bug and follow up in such detail.