[gwt-contrib] Change in gwt[master]: Set display to none for table columns that should not be vis...
Goktug Gokdogan has submitted this change and it was merged.
Change subject: Set display to none for table columns that should not be
visible.
..
Set display to none for table columns that should not be visible.
Change-Id: Id5eabeda8a3ef2f7b851d540c3776026984654c3
Review-Link: https://gwt-review.googlesource.com/#/c/3390/
---
M user/src/com/google/gwt/user/cellview/client/CellTable.java
M user/test/com/google/gwt/user/cellview/client/CellTableTest.java
2 files changed, 14 insertions(+), 5 deletions(-)
Approvals:
Leeroy Jenkins: Verified
Goktug Gokdogan: Looks good to me, approved
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 12b3db2..a3a1d57 100644
--- a/user/src/com/google/gwt/user/cellview/client/CellTable.java
+++ b/user/src/com/google/gwt/user/cellview/client/CellTable.java
@@ -18,6 +18,7 @@
import com.google.gwt.core.client.GWT;
import com.google.gwt.dom.client.BrowserEvents;
import com.google.gwt.dom.client.Document;
+import com.google.gwt.dom.client.Style.Display;
import com.google.gwt.dom.client.Style.TableLayout;
import com.google.gwt.dom.client.Style.Unit;
import com.google.gwt.dom.client.TableCellElement;
@@ -889,14 +890,21 @@
super.refreshColumnWidths();
/*
- * Set the width to zero 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.
+ * 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.
*/
if (colGroupEnabled) {
int colCount = colgroup.getChildCount();
- for (int i = getRealColumnCount(); i < colCount; i++) {
+ 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);
}
}
}
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 c54266f..965e59a 100644
--- a/user/test/com/google/gwt/user/cellview/client/CellTableTest.java
+++ b/user/test/com/google/gwt/user/cellview/client/CellTableTest.java
@@ -283,7 +283,7 @@
}
/**
- * Test that removing a column sets its width to zero.
+ * Test that removing a column sets its width to zero and the display to
none.
*/
public void testRemoveColumnWithWidth() {
CellTable table = createAbstractHasData(new TextCell());
@@ -297,6 +297,7 @@
table.removeColumn(column1);
table.getPresenter().flush();
assertEquals("0px", col1.getStyle().getWidth());
+assertEquals("none", col1.getStyle().getDisplay().toLowerCase());
}
@Override
--
To view, visit https://gwt-review.googlesource.com/3390
To unsubscribe, visit https://gwt-review.googlesource.com/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Id5eabeda8a3ef2f7b851d540c3776026984654c3
Gerrit-PatchSet: 3
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Aliseya Wright
Gerrit-Reviewer: Aliseya Wright
Gerrit-Reviewer: Goktug Gokdogan
Gerrit-Reviewer: Leeroy Jenkins
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
---
You received this message because you are subscribed to the Google Groups "GWT Contributors" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
[gwt-contrib] Change in gwt[master]: Set display to none for table columns that should not be vis...
Goktug Gokdogan has posted comments on this change. Change subject: Set display to none for table columns that should not be visible. .. Patch Set 3: Code-Review+2 Ok, I (or him) might have misinterpreted what is going on. If toggling columns works in practice, let's go ahead and see if it breaks anybody. (You can click submit now if you are ready to merge.) -- To view, visit https://gwt-review.googlesource.com/3390 To unsubscribe, visit https://gwt-review.googlesource.com/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id5eabeda8a3ef2f7b851d540c3776026984654c3 Gerrit-PatchSet: 3 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Aliseya Wright Gerrit-Reviewer: Aliseya Wright Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: Leeroy Jenkins Gerrit-HasComments: No -- http://groups.google.com/group/Google-Web-Toolkit-Contributors --- You received this message because you are subscribed to the Google Groups "GWT Contributors" group. To unsubscribe from this group and stop receiving emails from it, send an email to google-web-toolkit-contributors+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/groups/opt_out.
[gwt-contrib] Change in gwt[master]: Set display to none for table columns that should not be vis...
Goktug Gokdogan has posted comments on this change. Change subject: Set display to none for table columns that should not be visible. .. Patch Set 1: (1 comment) File user/src/com/google/gwt/user/cellview/client/CellTable.java Line 905: ensureTableColElement(i).getStyle().setDisplay(Display.NONE); Actually I'm referring to what he originally said but I'm talking about some other potential problems. In the original email (you might have not seen it) Alan said, "...it makes changes CSS attribute "display" to "none", and the column element loses its "table-column" properties, and its "width" attributes do not propagate to the and elements (because this element is not a table-column and does not describe a column of cells)..." so I thought that with your change we will have a problem with columns transitioning from hidden to visible. If the column was hidden and made just visible again the "setWidth" calls from super.refreshColumnWidths() will not be propagated to the cells because "display" property is not a "table-column". -- To view, visit https://gwt-review.googlesource.com/3390 To unsubscribe, visit https://gwt-review.googlesource.com/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id5eabeda8a3ef2f7b851d540c3776026984654c3 Gerrit-PatchSet: 1 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Aliseya Wright Gerrit-Reviewer: Aliseya Wright Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: Leeroy Jenkins Gerrit-HasComments: Yes -- http://groups.google.com/group/Google-Web-Toolkit-Contributors --- You received this message because you are subscribed to the Google Groups "GWT Contributors" group. To unsubscribe from this group and stop receiving emails from it, send an email to google-web-toolkit-contributors+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/groups/opt_out.
[gwt-contrib] Change in gwt[master]: Set display to none for table columns that should not be vis...
Goktug Gokdogan has posted comments on this change. Change subject: Set display to none for table columns that should not be visible. .. Patch Set 3: Aliseya, you very likely responded to my comments but I can't see them. In order to publish them, you need to press "review" and publish in the specific patch set that you responded. -- To view, visit https://gwt-review.googlesource.com/3390 To unsubscribe, visit https://gwt-review.googlesource.com/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id5eabeda8a3ef2f7b851d540c3776026984654c3 Gerrit-PatchSet: 3 Gerrit-Project: gwt Gerrit-Branch: master Gerrit-Owner: Aliseya Wright Gerrit-Reviewer: Goktug Gokdogan Gerrit-Reviewer: Leeroy Jenkins Gerrit-HasComments: No -- http://groups.google.com/group/Google-Web-Toolkit-Contributors --- You received this message because you are subscribed to the Google Groups "GWT Contributors" group. To unsubscribe from this group and stop receiving emails from it, send an email to google-web-toolkit-contributors+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/groups/opt_out.
[gwt-contrib] Change in gwt[master]: Set display to none for table columns that should not be vis...
Goktug Gokdogan has posted comments on this change.
Change subject: Set display to none for table columns that should not be
visible.
..
Patch Set 1:
(3 comments)
File user/src/com/google/gwt/user/cellview/client/CellTable.java
Line 905:
ensureTableColElement(i).getStyle().setDisplay(Display.NONE);
I think this has the same issue as before isn't it?
If I correctly understood the previous issue, if the column display is set
to none, the width was not propagating.
That will still be true for columns that was not visible before (hence
display is set to none) and become just visible. Their width will be set in
super.refreshColumnWidth when the display is still 'none'.
Line 909: }
Let's do 2 loops and avoid the unnecessary if checks:
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);
}
File user/test/com/google/gwt/user/cellview/client/CellTableTest.java
Line 301: assertEquals(Display.NONE.toString().toLowerCase(),
you can just change this to "none" if you like.
--
To view, visit https://gwt-review.googlesource.com/3390
To unsubscribe, visit https://gwt-review.googlesource.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id5eabeda8a3ef2f7b851d540c3776026984654c3
Gerrit-PatchSet: 1
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Aliseya Wright
Gerrit-Reviewer: Goktug Gokdogan
Gerrit-Reviewer: Leeroy Jenkins
Gerrit-HasComments: Yes
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
---
You received this message because you are subscribed to the Google Groups "GWT Contributors" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
