From 2c2b9e4d7eebedc7f8891c65d0d54322e2d068a8 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Johannes=20Dahlstr=C3=B6m?= Date: Wed, 4 Jun 2014 16:22:11 +0300 Subject: [PATCH] Notify EscalatorUpdater when removing columns (#13334) Change-Id: I8a598c195d13273d9adbcd13539f429733f9a34c --- .../com/vaadin/client/ui/grid/Escalator.java | 31 +++++++-- .../vaadin/client/ui/grid/FlyweightCell.java | 2 +- .../vaadin/client/ui/grid/FlyweightRow.java | 65 ++++++++++++------- 3 files changed, 71 insertions(+), 27 deletions(-) diff --git a/client/src/com/vaadin/client/ui/grid/Escalator.java b/client/src/com/vaadin/client/ui/grid/Escalator.java index 618e576aa6..c5b8cf0d79 100644 --- a/client/src/com/vaadin/client/ui/grid/Escalator.java +++ b/client/src/com/vaadin/client/ui/grid/Escalator.java @@ -1492,13 +1492,36 @@ public class Escalator extends Widget { final NodeList childNodes = root.getChildNodes(); for (int visualRowIndex = 0; visualRowIndex < childNodes .getLength(); visualRowIndex++) { - final Node tr = childNodes.getItem(visualRowIndex); + final Element tr = getTrByVisualIndex(visualRowIndex); - for (int column = 0; column < numberOfColumns; column++) { - Element cellElement = tr.getChild(offset).cast(); + flyweightRow.setup(tr, visualRowIndex, + columnConfiguration.getCalculatedColumnWidths()); + + Iterable cells = flyweightRow.getCells(offset, + numberOfColumns); + + getEscalatorUpdater().preDetach(flyweightRow, cells); + + for (FlyweightCell cell : cells) { + Element cellElement = cell.getElement(); detachPossibleWidgetFromCell(cellElement); cellElement.removeFromParent(); } + + /** + * We need a new iterable that does not try to reset the cell + * elements from the tr as they're not attached anymore. Instead + * the cells simply retain the now-unattached elements that were + * assigned on the above iteration. + * + * TODO a cleaner solution, eg. an iterable that only associates + * the elements once + */ + cells = flyweightRow + .getUnattachedCells(offset, numberOfColumns); + getEscalatorUpdater().postDetach(flyweightRow, cells); + + assert flyweightRow.teardown(); } reapplyRowWidths(); @@ -1616,7 +1639,7 @@ public class Escalator extends Widget { flyweightRow.setup(tr, logicalRowIndex, columnConfiguration.getCalculatedColumnWidths()); - Iterable cells = flyweightRow.getUninitializedCells( + Iterable cells = flyweightRow.getUnattachedCells( offset, numberOfCells); final int rowHeight = getDefaultRowHeight(); diff --git a/client/src/com/vaadin/client/ui/grid/FlyweightCell.java b/client/src/com/vaadin/client/ui/grid/FlyweightCell.java index 90a9ef4296..1b956258ff 100644 --- a/client/src/com/vaadin/client/ui/grid/FlyweightCell.java +++ b/client/src/com/vaadin/client/ui/grid/FlyweightCell.java @@ -99,7 +99,7 @@ public class FlyweightCell { void setup(final CellIterator iterator) { currentIterator = iterator; - if (iterator.areCellsInitialized()) { + if (iterator.areCellsAttached()) { final Element e = (Element) row.getElement().getChild(column); e.setPropertyInt(COLSPAN_ATTR, 1); e.getStyle().setWidth(row.getColumnWidth(column), Unit.PX); diff --git a/client/src/com/vaadin/client/ui/grid/FlyweightRow.java b/client/src/com/vaadin/client/ui/grid/FlyweightRow.java index 76013087f7..deaa5e2e3e 100644 --- a/client/src/com/vaadin/client/ui/grid/FlyweightRow.java +++ b/client/src/com/vaadin/client/ui/grid/FlyweightRow.java @@ -38,42 +38,41 @@ class FlyweightRow implements Row { static class CellIterator implements Iterator { /** A defensive copy of the cells in the current row. */ private final ArrayList cells; - private final boolean initialized; + private final boolean cellsAttached; private int cursor = 0; private int skipNext = 0; /** - * Creates a new iterator of initialized flyweight cells. A cell is - * initialized if it has a corresponding - * {@link FlyweightCell#getElement() DOM element} attached to the row - * element. + * Creates a new iterator of attached flyweight cells. A cell is + * attached if it has a corresponding {@link FlyweightCell#getElement() + * DOM element} attached to the row element. * * @param cells * the collection of cells to iterate */ - public static CellIterator initialized( + public static CellIterator attached( final Collection cells) { return new CellIterator(cells, true); } /** - * Creates a new iterator of uninitialized flyweight cells. A cell is - * uninitialized if it does not have a corresponding + * Creates a new iterator of unattached flyweight cells. A cell is + * unattached if it does not have a corresponding * {@link FlyweightCell#getElement() DOM element} attached to the row * element. * * @param cells * the collection of cells to iterate */ - public static CellIterator uninitialized( + public static CellIterator unattached( final Collection cells) { return new CellIterator(cells, false); } private CellIterator(final Collection cells, - final boolean initialized) { + final boolean attached) { this.cells = new ArrayList(cells); - this.initialized = initialized; + cellsAttached = attached; } @Override @@ -129,8 +128,8 @@ class FlyweightRow implements Row { return cells.subList(from, to); } - public boolean areCellsInitialized() { - return initialized; + public boolean areCellsAttached() { + return cellsAttached; } } @@ -215,7 +214,11 @@ class FlyweightRow implements Row { } /** - * Returns flyweight cells for the client code to render. + * Returns flyweight cells for the client code to render. The cells get + * their associated {@link FlyweightCell#getElement() elements} from the row + * element. + *

+ * Precondition: each cell has a corresponding element in the row * * @return an iterable of flyweight cells * @@ -223,21 +226,39 @@ class FlyweightRow implements Row { * @see #teardown() */ Iterable getCells() { + return getCells(0, cells.size()); + } + + /** + * Returns a subrange of flyweight cells for the client code to render. The + * cells get their associated {@link FlyweightCell#getElement() elements} + * from the row element. + *

+ * Precondition: each cell has a corresponding element in the row + * + * @param offset + * the index of the first cell to return + * @param numberOfCells + * the number of cells to return + * @return an iterable of flyweight cells + */ + Iterable getCells(final int offset, final int numberOfCells) { assertSetup(); return new Iterable() { @Override public Iterator iterator() { - return CellIterator.initialized(cells); + return CellIterator.attached(cells.subList(offset, offset + + numberOfCells)); } }; } /** - * Returns a subsequence of uninitialized flyweight cells. Uninitialized - * cells do not have {@link FlyweightCell#getElement() elements} associated. - * Note that FlyweightRow does not keep track of whether cells in actuality - * have corresponding DOM elements or not; it is the caller's responsibility - * to invoke this method with correct parameters. + * Returns a subrange of unattached flyweight cells. Unattached cells do + * not have {@link FlyweightCell#getElement() elements} associated. Note + * that FlyweightRow does not keep track of whether cells in actuality have + * corresponding DOM elements or not; it is the caller's responsibility to + * invoke this method with correct parameters. *

* Precondition: the range [offset, offset + numberOfCells) must be valid * @@ -247,14 +268,14 @@ class FlyweightRow implements Row { * the number of cells to return * @return an iterable of flyweight cells */ - Iterable getUninitializedCells(final int offset, + Iterable getUnattachedCells(final int offset, final int numberOfCells) { assertSetup(); assert offset >= 0 && offset + numberOfCells <= cells.size() : "Invalid range of cells"; return new Iterable() { @Override public Iterator iterator() { - return CellIterator.uninitialized(cells.subList(offset, offset + return CellIterator.unattached(cells.subList(offset, offset + numberOfCells)); } }; -- 2.39.5