Revision: 10376
Author: jlaba...@google.com
Date: Wed Jun 22 06:01:15 2011
Log: Fixing a bug in CellTree where pushing new data that renders to
the same content results in an invalid state where child are still attached
to a stale parent. For example, consider a tree contains IDs that
represent objects, but renders the ID using the objects display name. If
two objects with different IDs have the same display name, those two
objects will be rendered the same, which CellTree would optimize out. Now,
we perform the detach/reattach logic for child nodes, but we still do not
reset the innerHTML.
Sorry for the auto-format and Java 1.6 @Override spam. This is actually a
fairly small change, but I reformatted the files to get rid of checkstyle
warnings while I was editing them.
Review at http://gwt-code-reviews.appspot.com/1466803
Review by: tilma...@google.com
http://code.google.com/p/google-web-toolkit/source/detail?r=10376
Modified:
/trunk/user/src/com/google/gwt/user/cellview/client/AbstractHasData.java
/trunk/user/src/com/google/gwt/user/cellview/client/CellTreeNodeView.java
/trunk/user/src/com/google/gwt/user/cellview/client/HasDataPresenter.java
/trunk/user/test/com/google/gwt/user/cellview/client/CellTreeTest.java
/trunk/user/test/com/google/gwt/user/cellview/client/HasDataPresenterTest.java
=======================================
---
/trunk/user/src/com/google/gwt/user/cellview/client/AbstractHasData.java
Thu May 26 04:44:13 2011
+++
/trunk/user/src/com/google/gwt/user/cellview/client/AbstractHasData.java
Wed Jun 22 06:01:15 2011
@@ -72,18 +72,26 @@
this.hasData = hasData;
}
+ @Override
public <H extends EventHandler> HandlerRegistration addHandler(H
handler,
Type<H> type) {
return hasData.addHandler(handler, type);
}
+ @Override
public void render(SafeHtmlBuilder sb, List<T> values, int start,
SelectionModel<? super T> selectionModel) {
hasData.renderRowValues(sb, values, start, selectionModel);
}
+ @Override
public void replaceAllChildren(List<T> values, SafeHtml html,
- boolean stealFocus) {
+ boolean stealFocus, boolean contentChanged) {
+ if (!contentChanged) {
+ // Early exit if the content is unchanged.
+ return;
+ }
+
// Removing elements can fire a blur event, which we ignore.
hasData.isFocused = hasData.isFocused || stealFocus;
wasFocused = hasData.isFocused;
@@ -93,6 +101,7 @@
fireValueChangeEvent();
}
+ @Override
public void replaceChildren(List<T> values, int start, SafeHtml html,
boolean stealFocus) {
// Removing elements can fire a blur event, which we ignore.
@@ -104,9 +113,11 @@
fireValueChangeEvent();
}
+ @Override
public void resetFocus() {
if (wasFocused) {
CellBasedWidgetImpl.get().resetFocus(new
Scheduler.ScheduledCommand() {
+ @Override
public void execute() {
if (!hasData.resetFocusOnCell()) {
Element elem = hasData.getKeyboardSelectedElement();
@@ -119,12 +130,14 @@
}
}
+ @Override
public void setKeyboardSelected(int index, boolean seleted,
boolean stealFocus) {
hasData.isFocused = hasData.isFocused || stealFocus;
hasData.setKeyboardSelected(index, seleted, stealFocus);
}
+ @Override
public void setLoadingState(LoadingState state) {
hasData.isRefreshing = true;
hasData.onLoadingStateChanged(state);
@@ -299,6 +312,7 @@
selectionManagerReg =
addCellPreviewHandler(DefaultSelectionEventManager.<T>
createDefaultManager());
}
+ @Override
public HandlerRegistration addCellPreviewHandler(
CellPreviewEvent.Handler<T> handler) {
return presenter.addCellPreviewHandler(handler);
@@ -316,11 +330,13 @@
return presenter.addLoadingStateChangeHandler(handler);
}
+ @Override
public HandlerRegistration addRangeChangeHandler(
RangeChangeEvent.Handler handler) {
return presenter.addRangeChangeHandler(handler);
}
+ @Override
public HandlerRegistration addRowCountChangeHandler(
RowCountChangeEvent.Handler handler) {
return presenter.addRowCountChangeHandler(handler);
@@ -361,14 +377,17 @@
return getVisibleItems();
}
+ @Override
public KeyboardPagingPolicy getKeyboardPagingPolicy() {
return presenter.getKeyboardPagingPolicy();
}
+ @Override
public KeyboardSelectionPolicy getKeyboardSelectionPolicy() {
return presenter.getKeyboardSelectionPolicy();
}
+ @Override
public ProvidesKey<T> getKeyProvider() {
return presenter.getKeyProvider();
}
@@ -408,23 +427,28 @@
return getChildContainer();
}
+ @Override
public int getRowCount() {
return presenter.getRowCount();
}
+ @Override
public SelectionModel<? super T> getSelectionModel() {
return presenter.getSelectionModel();
}
+ @Override
public int getTabIndex() {
return tabIndex;
}
+ @Override
public T getVisibleItem(int indexOnPage) {
checkRowBounds(indexOnPage);
return presenter.getVisibleItem(indexOnPage);
}
+ @Override
public int getVisibleItemCount() {
return presenter.getVisibleItemCount();
}
@@ -435,14 +459,17 @@
*
* @return a List of displayed items
*/
+ @Override
public List<T> getVisibleItems() {
return presenter.getVisibleItems();
}
+ @Override
public Range getVisibleRange() {
return presenter.getVisibleRange();
}
+ @Override
public boolean isRowCountExact() {
return presenter.isRowCountExact();
}
@@ -536,11 +563,13 @@
*
* @see #getAccessKey()
*/
+ @Override
public void setAccessKey(char key) {
this.accessKey = key;
setKeyboardSelected(getKeyboardSelectedRow(), true, false);
}
+ @Override
public void setFocus(boolean focused) {
Element elem = getKeyboardSelectedElement();
if (elem != null) {
@@ -552,10 +581,12 @@
}
}
+ @Override
public void setKeyboardPagingPolicy(KeyboardPagingPolicy policy) {
presenter.setKeyboardPagingPolicy(policy);
}
+ @Override
public void setKeyboardSelectionPolicy(KeyboardSelectionPolicy policy) {
presenter.setKeyboardSelectionPolicy(policy);
}
@@ -584,10 +615,12 @@
setVisibleRange(pageStart, getPageSize());
}
+ @Override
public final void setRowCount(int count) {
setRowCount(count, true);
}
+ @Override
public void setRowCount(int size, boolean isExact) {
presenter.setRowCount(size, isExact);
}
@@ -611,6 +644,7 @@
setRowData(0, values);
}
+ @Override
public void setRowData(int start, List<? extends T> values) {
presenter.setRowData(start, values);
}
@@ -633,6 +667,7 @@
* com.google.gwt.view.client.CellPreviewEvent.Handler)
* @see #getSelectionModel()
*/
+ @Override
public void setSelectionModel(SelectionModel<? super T> selectionModel) {
presenter.setSelectionModel(selectionModel);
}
@@ -662,19 +697,23 @@
setSelectionModel(selectionModel);
}
+ @Override
public void setTabIndex(int index) {
this.tabIndex = index;
setKeyboardSelected(getKeyboardSelectedRow(), true, false);
}
+ @Override
public final void setVisibleRange(int start, int length) {
setVisibleRange(new Range(start, length));
}
+ @Override
public void setVisibleRange(Range range) {
presenter.setVisibleRange(range);
}
+ @Override
public void setVisibleRangeAndClearData(Range range,
boolean forceRangeChangeEvent) {
presenter.setVisibleRangeAndClearData(range, forceRangeChangeEvent);
=======================================
---
/trunk/user/src/com/google/gwt/user/cellview/client/CellTreeNodeView.java
Mon Jun 20 07:33:26 2011
+++
/trunk/user/src/com/google/gwt/user/cellview/client/CellTreeNodeView.java
Wed Jun 22 06:01:15 2011
@@ -192,7 +192,7 @@
@Override
public void replaceAllChildren(List<C> values, SafeHtml html,
- boolean stealFocus) {
+ boolean stealFocus, boolean contentChanged) {
// Hide the child container so we can animate it.
if (nodeView.tree.isAnimationEnabled()) {
nodeView.ensureAnimationFrame().getStyle().setDisplay(Display.NONE);
@@ -201,7 +201,9 @@
// Replace the child nodes.
nodeView.tree.isRefreshing = true;
Map<Object, CellTreeNodeView<?>> savedViews =
saveChildState(values, 0);
- AbstractHasData.replaceAllChildren(nodeView.tree, childContainer,
html);
+ if (contentChanged) {
+ AbstractHasData.replaceAllChildren(nodeView.tree,
childContainer, html);
+ }
nodeView.tree.isRefreshing = false;
// Trim the list of children.
=======================================
---
/trunk/user/src/com/google/gwt/user/cellview/client/HasDataPresenter.java
Mon Jun 20 07:33:26 2011
+++
/trunk/user/src/com/google/gwt/user/cellview/client/HasDataPresenter.java
Wed Jun 22 06:01:15 2011
@@ -66,8 +66,7 @@
*
* @param <T> the data type of items in the list
*/
-class HasDataPresenter<T> implements HasData<T>, HasKeyProvider<T>,
- HasKeyboardPagingPolicy {
+class HasDataPresenter<T> implements HasData<T>, HasKeyProvider<T>,
HasKeyboardPagingPolicy {
/**
* An iterator over DOM elements.
@@ -96,8 +95,7 @@
* @param handler the handler to add
* @param type the event type
*/
- <H extends EventHandler> HandlerRegistration addHandler(final H
handler,
- GwtEvent.Type<H> type);
+ <H extends EventHandler> HandlerRegistration addHandler(final H
handler, GwtEvent.Type<H> type);
/**
* Construct the HTML that represents the list of values, taking the
@@ -117,8 +115,12 @@
* @param values the values of the new children
* @param html the html to render in the child
* @param stealFocus true if the row should steal focus, false if not
+ * @param contentChanged indicates whether or not the content has
changed
+ * since the last call. If the content has not changed,
widgets can
+ * choose not to rerender themselves
*/
- void replaceAllChildren(List<T> values, SafeHtml html, boolean
stealFocus);
+ void replaceAllChildren(List<T> values, SafeHtml html, boolean
stealFocus,
+ boolean contentChanged);
/**
* Convert the specified HTML into DOM elements and replace the
existing
@@ -131,8 +133,7 @@
* @param html the HTML to convert
* @param stealFocus true if the row should steal focus, false if not
*/
- void replaceChildren(List<T> values, int start, SafeHtml html,
- boolean stealFocus);
+ void replaceChildren(List<T> values, int start, SafeHtml html, boolean
stealFocus);
/**
* Re-establish focus on an element within the view if the view
already had
@@ -463,8 +464,7 @@
* @param view the view implementation
* @param pageSize the default page size
*/
- public HasDataPresenter(HasData<T> display, View<T> view, int pageSize,
- ProvidesKey<T> keyProvider) {
+ public HasDataPresenter(HasData<T> display, View<T> view, int pageSize,
ProvidesKey<T> keyProvider) {
this.display = display;
this.view = view;
this.keyProvider = keyProvider;
@@ -472,25 +472,21 @@
}
@Override
- public HandlerRegistration addCellPreviewHandler(
- CellPreviewEvent.Handler<T> handler) {
+ public HandlerRegistration
addCellPreviewHandler(CellPreviewEvent.Handler<T> handler) {
return view.addHandler(handler, CellPreviewEvent.getType());
}
- public HandlerRegistration addLoadingStateChangeHandler(
- LoadingStateChangeEvent.Handler handler) {
+ public HandlerRegistration
addLoadingStateChangeHandler(LoadingStateChangeEvent.Handler handler) {
return view.addHandler(handler, LoadingStateChangeEvent.TYPE);
}
@Override
- public HandlerRegistration addRangeChangeHandler(
- RangeChangeEvent.Handler handler) {
+ public HandlerRegistration
addRangeChangeHandler(RangeChangeEvent.Handler handler) {
return view.addHandler(handler, RangeChangeEvent.getType());
}
@Override
- public HandlerRegistration addRowCountChangeHandler(
- RowCountChangeEvent.Handler handler) {
+ public HandlerRegistration
addRowCountChangeHandler(RowCountChangeEvent.Handler handler) {
return view.addHandler(handler, RowCountChangeEvent.getType());
}
@@ -559,8 +555,8 @@
* @return the row index, or -1 if disabled
*/
public int getKeyboardSelectedRow() {
- return KeyboardSelectionPolicy.DISABLED == keyboardSelectionPolicy ? -1
- : getCurrentState().getKeyboardSelectedRow();
+ return KeyboardSelectionPolicy.DISABLED == keyboardSelectionPolicy ?
-1 : getCurrentState()
+ .getKeyboardSelectedRow();
}
/**
@@ -571,8 +567,8 @@
* @return the row index, or -1 if disabled
*/
public int getKeyboardSelectedRowInView() {
- return KeyboardSelectionPolicy.DISABLED == keyboardSelectionPolicy ? -1
- : state.getKeyboardSelectedRow();
+ return KeyboardSelectionPolicy.DISABLED == keyboardSelectionPolicy ?
-1 : state
+ .getKeyboardSelectedRow();
}
/**
@@ -581,8 +577,8 @@
* @return the value, or null if a value was not selected
*/
public T getKeyboardSelectedRowValue() {
- return KeyboardSelectionPolicy.DISABLED == keyboardSelectionPolicy ?
null
- : getCurrentState().getKeyboardSelectedRowValue();
+ return KeyboardSelectionPolicy.DISABLED == keyboardSelectionPolicy ?
null : getCurrentState()
+ .getKeyboardSelectedRowValue();
}
@Override
@@ -727,8 +723,7 @@
// 0th index of next page.
setKeyboardSelectedRow(getPageSize(), true, false);
} else if (KeyboardPagingPolicy.INCREASE_RANGE ==
keyboardPagingPolicy) {
- setKeyboardSelectedRow(getKeyboardSelectedRow() + PAGE_INCREMENT,
true,
- false);
+ setKeyboardSelectedRow(getKeyboardSelectedRow() + PAGE_INCREMENT,
true, false);
}
}
@@ -749,8 +744,7 @@
// 0th index of previous page.
setKeyboardSelectedRow(-getPageSize(), true, false);
} else if (KeyboardPagingPolicy.INCREASE_RANGE ==
keyboardPagingPolicy) {
- setKeyboardSelectedRow(getKeyboardSelectedRow() - PAGE_INCREMENT,
true,
- false);
+ setKeyboardSelectedRow(getKeyboardSelectedRow() - PAGE_INCREMENT,
true, false);
}
}
@@ -777,8 +771,7 @@
* @param stealFocus true to steal focus
* @param forceUpdate force the update even if the row didn't change
*/
- public void setKeyboardSelectedRow(int index, boolean stealFocus,
- boolean forceUpdate) {
+ public void setKeyboardSelectedRow(int index, boolean stealFocus,
boolean forceUpdate) {
// Early exit if disabled.
if (KeyboardSelectionPolicy.DISABLED == keyboardSelectionPolicy) {
return;
@@ -791,8 +784,7 @@
* Early exit if the keyboard selected row has not changed and the
keyboard
* selected value is already set.
*/
- if (!forceUpdate && getKeyboardSelectedRow() == index
- && getKeyboardSelectedRowValue() != null) {
+ if (!forceUpdate && getKeyboardSelectedRow() == index &&
getKeyboardSelectedRowValue() != null) {
return;
}
@@ -818,8 +810,8 @@
pending.keyboardSelectedRowChanged = true;
if (index >= 0 && index < pageSize) {
pending.keyboardSelectedRow = index;
- pending.keyboardSelectedRowValue = index < pending.getRowDataSize()
- ? ensurePendingState().getRowDataValue(index) : null;
+ pending.keyboardSelectedRowValue =
+ index < pending.getRowDataSize() ?
ensurePendingState().getRowDataValue(index) : null;
pending.keyboardStealFocus = stealFocus;
return;
} else if (KeyboardPagingPolicy.CHANGE_PAGE == keyboardPagingPolicy) {
@@ -917,8 +909,7 @@
// Create placeholders up to the specified index.
PendingState<T> pending = ensurePendingState();
- int cacheOffset = Math.max(0, boundedStart - pageStart
- - getVisibleItemCount());
+ int cacheOffset = Math.max(0, boundedStart - pageStart -
getVisibleItemCount());
for (int i = 0; i < cacheOffset; i++) {
pending.rowData.add(null);
}
@@ -950,13 +941,14 @@
// Set the new selection model.
this.selectionModel = selectionModel;
if (selectionModel != null) {
- selectionHandler = selectionModel.addSelectionChangeHandler(new
SelectionChangeEvent.Handler() {
- @Override
- public void onSelectionChange(SelectionChangeEvent event) {
- // Ensure that we resolve selection.
- ensurePendingState();
- }
- });
+ selectionHandler =
+ selectionModel.addSelectionChangeHandler(new
SelectionChangeEvent.Handler() {
+ @Override
+ public void onSelectionChange(SelectionChangeEvent event) {
+ // Ensure that we resolve selection.
+ ensurePendingState();
+ }
+ });
}
// Update the current selection state based on the new model.
@@ -979,8 +971,7 @@
}
@Override
- public void setVisibleRangeAndClearData(Range range,
- boolean forceRangeChangeEvent) {
+ public void setVisibleRangeAndClearData(Range range, boolean
forceRangeChangeEvent) {
setVisibleRange(range, true, forceRangeChangeEvent);
}
@@ -1009,8 +1000,7 @@
* @param modifiedRows the indexes of modified rows
* @return up to two ranges that encompass the modified rows
*/
- List<Range> calculateModifiedRanges(TreeSet<Integer> modifiedRows,
- int pageStart, int pageEnd) {
+ List<Range> calculateModifiedRanges(TreeSet<Integer> modifiedRows, int
pageStart, int pageEnd) {
int rangeStart0 = -1;
int rangeEnd0 = -1;
int rangeStart1 = -1;
@@ -1157,8 +1147,7 @@
* @return the key
*/
private Object getRowValueKey(T rowValue) {
- return (keyProvider == null || rowValue == null) ? rowValue
- : keyProvider.getKey(rowValue);
+ return (keyProvider == null || rowValue == null) ? rowValue :
keyProvider.getKey(rowValue);
}
/**
@@ -1195,8 +1184,7 @@
throw new IllegalStateException(
"The Cell Widget is attempting to render itself within the
render "
+ "loop. This usually happens when your render code modifies
the "
- + "state of the Cell Widget then accesses data or elements "
- + "within the Widget.");
+ + "state of the Cell Widget then accesses data or elements "
+ "within the Widget.");
}
isResolvingState = true;
@@ -1216,25 +1204,26 @@
* If the row value exists in multiple places, use the closest index.
If the
* row value not longer exists, use the current index.
*/
- pending.keyboardSelectedRow = Math.max(0,
- Math.min(pending.keyboardSelectedRow, rowDataCount - 1));
+ pending.keyboardSelectedRow =
+ Math.max(0, Math.min(pending.keyboardSelectedRow, rowDataCount -
1));
if (KeyboardSelectionPolicy.DISABLED == keyboardSelectionPolicy) {
// Clear the keyboard selected state.
pending.keyboardSelectedRow = 0;
pending.keyboardSelectedRowValue = null;
} else if (pending.keyboardSelectedRowChanged) {
// Choose the row value based on the index.
- pending.keyboardSelectedRowValue = rowDataCount > 0
- ? pending.getRowDataValue(pending.keyboardSelectedRow) : null;
+ pending.keyboardSelectedRowValue =
+ rowDataCount > 0 ?
pending.getRowDataValue(pending.keyboardSelectedRow) : null;
} else if (pending.keyboardSelectedRowValue != null) {
// Choose the index based on the row value.
- int bestMatchIndex = findIndexOfBestMatch(pending,
- pending.keyboardSelectedRowValue, pending.keyboardSelectedRow);
+ int bestMatchIndex =
+ findIndexOfBestMatch(pending, pending.keyboardSelectedRowValue,
+ pending.keyboardSelectedRow);
if (bestMatchIndex >= 0) {
// A match was found.
pending.keyboardSelectedRow = bestMatchIndex;
- pending.keyboardSelectedRowValue = rowDataCount > 0
- ? pending.getRowDataValue(pending.keyboardSelectedRow) : null;
+ pending.keyboardSelectedRowValue =
+ rowDataCount > 0 ?
pending.getRowDataValue(pending.keyboardSelectedRow) : null;
} else {
// No match was found, so reset to 0.
pending.keyboardSelectedRow = 0;
@@ -1253,8 +1242,8 @@
&& selectionModel != null && pending.viewTouched) {
T oldValue = oldState.getSelectedValue();
Object oldKey = getRowValueKey(oldValue);
- T newValue = rowDataCount > 0
- ? pending.getRowDataValue(pending.getKeyboardSelectedRow()) :
null;
+ T newValue =
+ rowDataCount > 0 ?
pending.getRowDataValue(pending.getKeyboardSelectedRow()) : null;
Object newKey = getRowValueKey(newValue);
/*
* Do not deselect the old value unless we have a new value to
select,
@@ -1264,10 +1253,10 @@
if (newKey != null && !newKey.equals(oldKey)) {
// Check both values for selection before setting selection, or
the
// selection model may resolve state early.
- boolean oldValueWasSelected = (oldValue == null) ? false
- : selectionModel.isSelected(oldValue);
- boolean newValueWasSelected = (newValue == null) ? false
- : selectionModel.isSelected(newValue);
+ boolean oldValueWasSelected =
+ (oldValue == null) ? false :
selectionModel.isSelected(oldValue);
+ boolean newValueWasSelected =
+ (newValue == null) ? false :
selectionModel.isSelected(newValue);
// Deselect the old value.
if (oldValueWasSelected) {
@@ -1288,9 +1277,10 @@
}
// If the keyboard row changes, add it to the modified set.
- boolean keyboardRowChanged = pending.keyboardSelectedRowChanged
- || (oldState.getKeyboardSelectedRow() !=
pending.keyboardSelectedRow)
- || (oldState.getKeyboardSelectedRowValue() == null &&
pending.keyboardSelectedRowValue != null);
+ boolean keyboardRowChanged =
+ pending.keyboardSelectedRowChanged
+ || (oldState.getKeyboardSelectedRow() !=
pending.keyboardSelectedRow)
+ || (oldState.getKeyboardSelectedRowValue() == null &&
pending.keyboardSelectedRowValue != null);
/*
* Resolve selection. Check the selection status of all row values in
the
@@ -1301,7 +1291,8 @@
for (int i = pageStart; i < pageStart + rowDataCount; i++) {
// Check the new selection state.
T rowValue = pending.getRowDataValue(i - pageStart);
- boolean isSelected = (rowValue != null && selectionModel != null &&
selectionModel.isSelected(rowValue));
+ boolean isSelected =
+ (rowValue != null && selectionModel != null &&
selectionModel.isSelected(rowValue));
// Compare to the old selection state.
boolean wasSelected = oldState.isRowSelected(i);
@@ -1350,8 +1341,7 @@
}
// Calculate the modified ranges.
- List<Range> modifiedRanges = calculateModifiedRanges(modifiedRows,
- pageStart, pageEnd);
+ List<Range> modifiedRanges = calculateModifiedRanges(modifiedRows,
pageStart, pageEnd);
Range range0 = modifiedRanges.size() > 0 ? modifiedRanges.get(0) :
null;
Range range1 = modifiedRanges.size() > 1 ? modifiedRanges.get(1) :
null;
int replaceDiff = 0; // The total number of rows to replace.
@@ -1372,13 +1362,11 @@
} else if (rowDataCount < oldRowDataCount) {
// Redraw if we have trimmed the row data.
redrawRequired = true;
- } else if (range1 == null && range0 != null
- && range0.getStart() == pageStart
+ } else if (range1 == null && range0 != null && range0.getStart() ==
pageStart
&& (replaceDiff >= oldRowDataCount || replaceDiff > oldPageSize)) {
// Redraw if the new data completely overlaps the old data.
redrawRequired = true;
- } else if (replaceDiff >= REDRAW_MINIMUM
- && replaceDiff > REDRAW_THRESHOLD * oldRowDataCount) {
+ } else if (replaceDiff >= REDRAW_MINIMUM && replaceDiff >
REDRAW_THRESHOLD * oldRowDataCount) {
/*
* Redraw if the number of modified rows represents a large portion
of the
* view, defined as greater than 30% of the rows (minimum of 5).
@@ -1405,11 +1393,10 @@
SafeHtmlBuilder sb = new SafeHtmlBuilder();
view.render(sb, pending.rowData, pending.pageStart,
selectionModel);
SafeHtml newContents = sb.toSafeHtml();
- if (!newContents.equals(lastContents)) {
- lastContents = newContents;
- view.replaceAllChildren(pending.rowData, newContents,
- pending.keyboardStealFocus);
- }
+ boolean contentChanged = !newContents.equals(lastContents);
+ lastContents = newContents;
+ view.replaceAllChildren(pending.rowData, newContents,
pending.keyboardStealFocus,
+ contentChanged);
view.resetFocus();
} else if (range0 != null) {
// Replace specific rows.
@@ -1420,11 +1407,9 @@
int absStart = range0.getStart();
int relStart = absStart - pageStart;
SafeHtmlBuilder sb = new SafeHtmlBuilder();
- List<T> replaceValues = pending.rowData.subList(relStart,
relStart
- + range0.getLength());
+ List<T> replaceValues = pending.rowData.subList(relStart,
relStart + range0.getLength());
view.render(sb, replaceValues, absStart, selectionModel);
- view.replaceChildren(replaceValues, relStart, sb.toSafeHtml(),
- pending.keyboardStealFocus);
+ view.replaceChildren(replaceValues, relStart, sb.toSafeHtml(),
pending.keyboardStealFocus);
}
// Replace range1 if it exists.
@@ -1432,11 +1417,9 @@
int absStart = range1.getStart();
int relStart = absStart - pageStart;
SafeHtmlBuilder sb = new SafeHtmlBuilder();
- List<T> replaceValues = pending.rowData.subList(relStart,
relStart
- + range1.getLength());
+ List<T> replaceValues = pending.rowData.subList(relStart,
relStart + range1.getLength());
view.render(sb, replaceValues, absStart, selectionModel);
- view.replaceChildren(replaceValues, relStart, sb.toSafeHtml(),
- pending.keyboardStealFocus);
+ view.replaceChildren(replaceValues, relStart, sb.toSafeHtml(),
pending.keyboardStealFocus);
}
view.resetFocus();
@@ -1451,8 +1434,7 @@
// Select the new keyboard row.
int newSelectedRow = pending.getKeyboardSelectedRow();
if (newSelectedRow >= 0 && newSelectedRow < rowDataCount) {
- view.setKeyboardSelected(newSelectedRow, true,
- pending.keyboardStealFocus);
+ view.setKeyboardSelected(newSelectedRow, true,
pending.keyboardStealFocus);
}
}
} catch (Error e) {
@@ -1476,8 +1458,7 @@
* @param clearData true to clear all data
* @param forceRangeChangeEvent true to force a {@link RangeChangeEvent}
*/
- private void setVisibleRange(Range range, boolean clearData,
- boolean forceRangeChangeEvent) {
+ private void setVisibleRange(Range range, boolean clearData, boolean
forceRangeChangeEvent) {
final int start = range.getStart();
final int length = range.getLength();
if (start < 0) {
@@ -1555,8 +1536,7 @@
*/
private void updateCachedData() {
int pageStart = getPageStart();
- int expectedLastIndex = Math.max(0,
- Math.min(getPageSize(), getRowCount() - pageStart));
+ int expectedLastIndex = Math.max(0, Math.min(getPageSize(),
getRowCount() - pageStart));
int lastIndex = getVisibleItemCount() - 1;
while (lastIndex >= expectedLastIndex) {
ensurePendingState().rowData.remove(lastIndex);
=======================================
--- /trunk/user/test/com/google/gwt/user/cellview/client/CellTreeTest.java
Mon Jun 20 07:33:26 2011
+++ /trunk/user/test/com/google/gwt/user/cellview/client/CellTreeTest.java
Wed Jun 22 06:01:15 2011
@@ -1,12 +1,12 @@
/*
* Copyright 2010 Google Inc.
- *
+ *
* Licensed under the Apache License, Version 2.0 (the "License"); you may
not
* use this file except in compliance with the License. You may obtain a
copy of
* the License at
- *
+ *
* http://www.apache.org/licenses/LICENSE-2.0
- *
+ *
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
@@ -15,10 +15,16 @@
*/
package com.google.gwt.user.cellview.client;
+import com.google.gwt.cell.client.AbstractCell;
import com.google.gwt.cell.client.TextCell;
+import com.google.gwt.safehtml.shared.SafeHtmlBuilder;
+import com.google.gwt.user.client.ui.RootPanel;
import com.google.gwt.view.client.ListDataProvider;
import com.google.gwt.view.client.TreeViewModel;
+import java.util.ArrayList;
+import java.util.List;
+
/**
* Tests for {@link CellTree}.
*/
@@ -47,12 +53,76 @@
// Create the tree.
CellTree tree = createAbstractCellTree(model, null);
tree.rootNode.listView.presenter.flush();
-
+
// Refresh the empty list.
provider.refresh();
provider.flush();
tree.rootNode.listView.presenter.flush();
}
+
+ /**
+ * Test that CellTree handles rendering the same content, but with a
different
+ * underlying value.
+ */
+ public void testRenderSameContent() {
+ final AbstractCell<Integer> intCell = new AbstractCell<Integer>() {
+ @Override
+ public void render(Context context, Integer value, SafeHtmlBuilder
sb) {
+ sb.append(value % 10); // Render the units digit only.
+ }
+ };
+
+ // Create a data provider for the root node.
+ final ListDataProvider<Integer> root = new ListDataProvider<Integer>();
+ for (int i = 0; i < 9; i++) {
+ root.getList().add(i);
+ }
+
+ TreeViewModel model = new TreeViewModel() {
+ @Override
+ public NodeInfo<?> getNodeInfo(Object value) {
+ if (value == null) {
+ // Return the root node.
+ return new DefaultNodeInfo<Integer>(root, intCell);
+ } else {
+ // Return a child node.
+ return new DefaultNodeInfo<String>(new
ListDataProvider<String>(), new TextCell());
+ }
+ }
+
+ @Override
+ public boolean isLeaf(Object value) {
+ return false;
+ }
+ };
+
+ CellTree tree = createAbstractCellTree(model, null);
+ RootPanel.get().add(tree);
+ tree.rootNode.listView.presenter.flush();
+
+ // Open the first child.
+ TreeNode rootNode = tree.getRootTreeNode();
+ assertEquals(1, rootNode.getChildValue(1));
+ TreeNode child1 = rootNode.setChildOpen(1, true);
+ assertFalse(child1.isDestroyed());
+ assertTrue(rootNode.isChildOpen(1));
+
+ // Replace all values in the list.
+ List<Integer> oldData = root.getList();
+ List<Integer> newData = new ArrayList<Integer>();
+ for (int l : oldData) {
+ newData.add(l + 100); // renders the same as the current value.
+ }
+ root.setList(newData);
+ root.flush();
+ tree.rootNode.listView.presenter.flush();
+
+ // Child1 is closed and destroyed.
+ assertFalse(rootNode.isChildOpen(1));
+ assertTrue(child1.isDestroyed());
+
+ RootPanel.get().remove(tree);
+ }
/**
* Test that replacing a subset of children updates both the TreeNode
value
@@ -80,7 +150,7 @@
assertEquals("ab", a.getChildValue(1));
assertEquals("new", newNode.getValue());
assertEquals("newc", newNode.getChildValue(2));
-
+
// Check the underlying DOM values.
CellTreeNodeView<?> aImpl = cellTree.rootNode.getChildNode(0);
CellTreeNodeView<?> newNodeImpl = cellTree.rootNode.getChildNode(1);
@@ -106,8 +176,7 @@
}
@Override
- protected <T> CellTree createAbstractCellTree(
- TreeViewModel model, T rootValue) {
+ protected <T> CellTree createAbstractCellTree(TreeViewModel model, T
rootValue) {
return new CellTree(model, rootValue);
}
}
=======================================
---
/trunk/user/test/com/google/gwt/user/cellview/client/HasDataPresenterTest.java
Tue Feb 8 04:44:50 2011
+++
/trunk/user/test/com/google/gwt/user/cellview/client/HasDataPresenterTest.java
Wed Jun 22 06:01:15 2011
@@ -68,6 +68,7 @@
eventFired = false;
}
+ @Override
public void onSelectionChange(SelectionChangeEvent event) {
assertFalse(eventFired);
eventFired = true;
@@ -105,8 +106,10 @@
private final List<SafeHtml> lastHtml = new ArrayList<SafeHtml>();
private LoadingState loadingState;
private boolean replaceAllChildrenCalled;
+ private boolean replaceAllChildrenCalledWithSameContent;
private boolean replaceChildrenCalled;
+ @Override
public <H extends EventHandler> HandlerRegistration addHandler(H
handler,
Type<H> type) {
throw new UnsupportedOperationException();
@@ -148,6 +151,12 @@
assertEquals(expected, replaceAllChildrenCalled);
replaceAllChildrenCalled = false;
}
+
+ public void assertReplaceAllChildrenCalled(boolean expected, boolean
sameContent) {
+ assertReplaceAllChildrenCalled(expected);
+ assertEquals(sameContent, replaceAllChildrenCalledWithSameContent);
+ replaceAllChildrenCalledWithSameContent = false;
+ }
public void assertReplaceChildrenCalled(boolean expected) {
assertEquals(expected, replaceChildrenCalled);
@@ -158,19 +167,23 @@
return childCount;
}
+ @Override
public void render(SafeHtmlBuilder sb, List<T> values, int start,
SelectionModel<? super T> selectionModel) {
sb.appendHtmlConstant("start=").append(start);
sb.appendHtmlConstant(",size=").append(values.size());
}
+ @Override
public void replaceAllChildren(List<T> values, SafeHtml html,
- boolean stealFocus) {
+ boolean stealFocus, boolean contentChanged) {
+ replaceAllChildrenCalledWithSameContent = !contentChanged;
childCount = values.size();
replaceAllChildrenCalled = true;
lastHtml.add(html);
}
+ @Override
public void replaceChildren(List<T> values, int start, SafeHtml html,
boolean stealFocus) {
childCount = Math.max(childCount, start + values.size());
@@ -178,15 +191,18 @@
lastHtml.add(html);
}
+ @Override
public void resetFocus() {
}
+ @Override
public void setKeyboardSelected(int index, boolean selected,
boolean stealFocus) {
keyboardSelectedRow.add(index);
keyboardSelectedRowState.add(selected);
}
+ @Override
public void setLoadingState(LoadingState state) {
this.loadingState = state;
}
@@ -280,21 +296,26 @@
*/
public void testBadViewSelectionModel() {
SelectionModel<String> badModel = new SelectionModel<String>() {
+ @Override
public void fireEvent(GwtEvent<?> event) {
}
+ @Override
public Object getKey(String item) {
return null;
}
+ @Override
public HandlerRegistration addSelectionChangeHandler(Handler
handler) {
return null;
}
+ @Override
public boolean isSelected(String object) {
throw new NullPointerException();
}
+ @Override
public void setSelected(String object, boolean selected) {
throw new NullPointerException();
}
@@ -338,7 +359,7 @@
MockView<String> badView = new MockView<String>() {
@Override
public void replaceAllChildren(List<String> values, SafeHtml html,
- boolean stealFocus) {
+ boolean stealFocus, boolean contentChanged) {
throw new NullPointerException();
}
@@ -889,6 +910,7 @@
// The pending command is scheduled. Wait for it to execute.
delayTestFinish(5000);
Scheduler.get().scheduleDeferred(new Scheduler.ScheduledCommand() {
+ @Override
public void execute() {
assertFalse(presenter.hasPendingState());
view.assertReplaceAllChildrenCalled(true);
@@ -1536,7 +1558,7 @@
presenter.setVisibleRange(new Range(0, 10));
presenter.setRowData(0, createData(0, 10));
presenter.flush();
- view.assertReplaceAllChildrenCalled(true);
+ view.assertReplaceAllChildrenCalled(true, false);
view.assertReplaceChildrenCalled(false);
view.assertLastHtml("start=0,size=10");
view.assertLoadingState(LoadingState.LOADED);
@@ -1544,9 +1566,9 @@
// Set the same data over the entire range.
presenter.setRowData(0, createData(0, 10));
presenter.flush();
- view.assertReplaceAllChildrenCalled(false);
+ view.assertReplaceAllChildrenCalled(true, true);
view.assertReplaceChildrenCalled(false);
- view.assertLastHtml(null);
+ view.assertLastHtml("start=0,size=10");
view.assertLoadingState(LoadingState.LOADED);
}
@@ -1669,6 +1691,7 @@
// Add a range change handler.
final List<Range> events = new ArrayList<Range>();
listView.addRangeChangeHandler(new RangeChangeEvent.Handler() {
+ @Override
public void onRangeChange(RangeChangeEvent event) {
events.add(event.getNewRange());
}
@@ -1707,6 +1730,7 @@
// Add a range change handler.
final List<Range> events = new ArrayList<Range>();
listView.addRangeChangeHandler(new RangeChangeEvent.Handler() {
+ @Override
public void onRangeChange(RangeChangeEvent event) {
events.add(event.getNewRange());
}
@@ -1744,6 +1768,7 @@
// Add a range change handler.
final List<Range> events = new ArrayList<Range>();
listView.addRangeChangeHandler(new RangeChangeEvent.Handler() {
+ @Override
public void onRangeChange(RangeChangeEvent event) {
events.add(event.getNewRange());
}
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
