When removing columns from CellTable, also remove the col element.
This fixes a bug in Firefox. Firefox would remove the
column, yet keep white space the size of the removed column instead of
having the table fill the remaining space.
Change-Id: I5651120d2c8bdd36d5c1787a9dbf6a0935750c2f
Review-Link: https://gwt-review.googlesource.com/#/c/12641/
diff --git a/user/src/com/google/gwt/user/cellview/client/CellTable.java b/user/src/com/google/gwt/user/cellview/client/CellTable.java
index 419a49f..8253cd6 100644
--- a/user/src/com/google/gwt/user/cellview/client/CellTable.java
+++ b/user/src/com/google/gwt/user/cellview/client/CellTable.java
@@ -502,6 +502,7 @@
private TableSectionElement tfoot;
private TableSectionElement thead;
private boolean colGroupEnabled = true;
+ private boolean removeColumnsOnHide = true;
/**
* Constructs a table with a default page size of 15.
@@ -581,11 +582,11 @@
Widget loadingIndicator) {
this(pageSize, resources, keyProvider, loadingIndicator, true, true);
}
-
+
/**
* Constructs a table with the specified page size, {@link Resources}, key
* provider, and loading indicator.
- *
+ *
* @param pageSize the page size
* @param resources the resources to use for this widget
* @param keyProvider an instance of ProvidesKey<T>, or null if the record
@@ -595,7 +596,7 @@
* @param enableColGroup enable colgroup element. This is used when the table is using fixed
* layout and when column style is added. Ignoring this element will boost rendering
* performance. Note that when colgroup is disabled, {@link #setColumnWidth},
- * {@link setTableLayoutFixed} and {@link addColumnStyleName} are no longe supported
+ * {@link setTableLayoutFixed} and {@link addColumnStyleName} are no longer supported
* @param attachLoadingPanel attaching the table section that contains the empty table widget and
* the loading indicator. Attaching this to the table significantly improve the rendering
* performance in webkit based browsers but also introduces significantly larger latency
@@ -818,6 +819,10 @@
setTableLayoutFixed(isFixedLayout);
}
+ public void setRemoveColumnsOnHide(boolean removeColumnsOnHide) {
+ this.removeColumnsOnHide = removeColumnsOnHide;
+ }
+
@Override
protected void doSetColumnWidth(int column, String width) {
// This is invoked when column width is set (which will throw an exception if colgroup is not
@@ -889,21 +894,22 @@
super.refreshColumnWidths();
/*
- * Set the width to zero and the display to none for all col elements that
- * appear after the last column. Clearing the width would cause it to take
- * up the remaining width in a fixed layout table.
- *
- * Clear the display for all columns that appear in the table.
+ * Clear the display for all columns that appear in the table. And removes any cols from the
+ * colgroup that are no longer in the table.
*/
if (colGroupEnabled) {
int colCount = colgroup.getChildCount();
- int lastColumn = getRealColumnCount();
+ int lastColumn = getRealColumnCount();
for (int i = 0; i < lastColumn; i++) {
ensureTableColElement(i).getStyle().clearDisplay();
}
- for (int i = lastColumn; i < colCount; i++) {
- doSetColumnWidth(i, "0px");
- ensureTableColElement(i).getStyle().setDisplay(Display.NONE);
+ for (int i = colCount - 1; i >= lastColumn; i--) {
+ if (removeColumnsOnHide) {
+ doSetColumnWidth(i, "0px");
+ ensureTableColElement(i).getStyle().setDisplay(Display.NONE);
+ } else {
+ colgroup.removeChild(colgroup.getChild(i));
+ }
}
}
}
diff --git a/user/test/com/google/gwt/user/cellview/client/CellTableTest.java b/user/test/com/google/gwt/user/cellview/client/CellTableTest.java
index 9bc9aaf..7dae8ee 100644
--- a/user/test/com/google/gwt/user/cellview/client/CellTableTest.java
+++ b/user/test/com/google/gwt/user/cellview/client/CellTableTest.java
@@ -291,17 +291,38 @@
/**
* Test that removing a column sets its width to zero and the display to none.
*/
- public void testRemoveColumnWithWidth() {
+ public void testRemoveColumnWithWidth_withRemoveColumnsOnHideDisabled() {
CellTable<String> table = createAbstractHasData(new TextCell());
+ table.setRemoveColumnsOnHide(false);
Column<String, ?> column1 = table.getColumn(1);
table.setColumnWidth(column1, "100px");
Element col0 = table.colgroup.getFirstChildElement();
Element col1 = col0.getNextSiblingElement();
assertEquals("100px", col1.getStyle().getWidth().toLowerCase(Locale.ROOT));
+ assertEquals(2, table.colgroup.getChildCount());
// Remove column 1.
table.removeColumn(column1);
table.getPresenter().flush();
+
+ assertNull(col0.getNextSiblingElement());
+ assertEquals(1, table.colgroup.getChildCount());
+ }
+
+ public void testRemoveColumnWithWidth_withRemoveColumnsOnHideEnabled() {
+ CellTable<String> table = createAbstractHasData(new TextCell());
+ table.setRemoveColumnsOnHide(true);
+ Column<String, ?> column1 = table.getColumn(1);
+ table.setColumnWidth(column1, "100px");
+ Element col0 = table.colgroup.getFirstChildElement();
+ Element col1 = col0.getNextSiblingElement();
+ assertEquals("100px", col1.getStyle().getWidth().toLowerCase(Locale.ROOT));
+ assertEquals(2, table.colgroup.getChildCount());
+
+ // Remove column 1.
+ table.removeColumn(column1);
+ table.getPresenter().flush();
+
assertEquals("0px", col1.getStyle().getWidth());
assertEquals("none", col1.getStyle().getDisplay().toLowerCase(Locale.ROOT));
}
