From 8a6c260a6a59a412227366d02e5cbce4108b2fd5 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Pekka=20Hyv=C3=B6nen?= Date: Mon, 13 Apr 2015 08:36:27 +0300 Subject: [PATCH] Move details row decorators out of spacers td #17423 Change-Id: Ie6c0166ad307b9172acccaa781d6fea316ee6390 --- WebContent/VAADIN/themes/base/grid/grid.scss | 19 ++- .../client/widget/escalator/Spacer.java | 5 + .../com/vaadin/client/widgets/Escalator.java | 125 ++++++++++++++- .../src/com/vaadin/client/widgets/Grid.java | 149 +++--------------- 4 files changed, 158 insertions(+), 140 deletions(-) diff --git a/WebContent/VAADIN/themes/base/grid/grid.scss b/WebContent/VAADIN/themes/base/grid/grid.scss index 360f020822..0e1dee2b99 100644 --- a/WebContent/VAADIN/themes/base/grid/grid.scss +++ b/WebContent/VAADIN/themes/base/grid/grid.scss @@ -1,4 +1,5 @@ -$v-grid-border: 1px solid #ddd !default; +$v-grid-border-size: 1px !default; +$v-grid-border: $v-grid-border-size solid #ddd !default; $v-grid-cell-vertical-border: $v-grid-border !default; $v-grid-cell-horizontal-border: $v-grid-cell-vertical-border !default; $v-grid-cell-focused-border: 1px solid !default; @@ -420,6 +421,10 @@ $v-grid-details-border-bottom-stripe: 1px solid darken($v-grid-row-background-co margin-right: 4px; } + .#{$primaryStyleName}-spacer { + left: $v-grid-details-marker-width - $v-grid-border-size; + } + .#{$primaryStyleName}-spacer > td { display: block; padding: 0; @@ -434,8 +439,14 @@ $v-grid-details-border-bottom-stripe: 1px solid darken($v-grid-row-background-co border-top: $v-grid-details-border-top-stripe; border-bottom: $v-grid-details-border-bottom-stripe; } + + .#{$primaryStyleName}-spacer-deco-container { + position: relative; + top: $v-grid-border-size; + z-index: 5; + } - .#{$primaryStyleName}-spacer .deco { + .#{$primaryStyleName}-spacer-deco { top: 0; // this will be overridden by code, but it's a good default. left: 0; width: $v-grid-details-marker-width; @@ -456,10 +467,6 @@ $v-grid-details-border-bottom-stripe: 1px solid darken($v-grid-row-background-co } } - .#{$primaryStyleName}-spacer .content { - padding-left: $v-grid-details-marker-width; - } - // Renderers .#{$primaryStyleName}-cell > .v-progressbar { diff --git a/client/src/com/vaadin/client/widget/escalator/Spacer.java b/client/src/com/vaadin/client/widget/escalator/Spacer.java index 6ccef88f0d..789a64a21e 100644 --- a/client/src/com/vaadin/client/widget/escalator/Spacer.java +++ b/client/src/com/vaadin/client/widget/escalator/Spacer.java @@ -33,6 +33,11 @@ public interface Spacer { */ Element getElement(); + /** + * Gets the decorative element for this spacer. + */ + Element getDecoElement(); + /** * Gets the row index. * diff --git a/client/src/com/vaadin/client/widgets/Escalator.java b/client/src/com/vaadin/client/widgets/Escalator.java index 01567143dd..eae5789b8a 100644 --- a/client/src/com/vaadin/client/widgets/Escalator.java +++ b/client/src/com/vaadin/client/widgets/Escalator.java @@ -962,6 +962,7 @@ public class Escalator extends Widget implements RequiresResize, lastScrollTop = scrollTop; body.updateEscalatorRowsOnScroll(); + body.spacerContainer.updateSpacerDecosVisibility(); /* * TODO [[optimize]]: Might avoid a reflow by first calculating new * scrolltop and scrolleft, then doing the escalator magic based on @@ -2195,6 +2196,7 @@ public class Escalator extends Widget implements RequiresResize, - footer.getHeightOfSection()); body.verifyEscalatorCount(); + body.spacerContainer.updateSpacerDecosVisibility(); } Profiler.leave("Escalator.AbstractStaticRowContainer.recalculateSectionHeight"); @@ -2258,10 +2260,13 @@ public class Escalator extends Widget implements RequiresResize, @Override protected void sectionHeightCalculated() { - bodyElem.getStyle().setMarginTop(getHeightOfSection(), Unit.PX); + double heightOfSection = getHeightOfSection(); + bodyElem.getStyle().setMarginTop(heightOfSection, Unit.PX); + spacerDecoContainer.getStyle().setMarginTop(heightOfSection, + Unit.PX); verticalScrollbar.getElement().getStyle() - .setTop(getHeightOfSection(), Unit.PX); - headerDeco.getStyle().setHeight(getHeightOfSection(), Unit.PX); + .setTop(heightOfSection, Unit.PX); + headerDeco.getStyle().setHeight(heightOfSection, Unit.PX); } @Override @@ -3473,6 +3478,7 @@ public class Escalator extends Widget implements RequiresResize, tBodyScrollLeft = scrollLeft; tBodyScrollTop = scrollTop; position.set(bodyElem, -tBodyScrollLeft, -tBodyScrollTop); + position.set(spacerDecoContainer, 0, -tBodyScrollTop); } /** @@ -4602,9 +4608,11 @@ public class Escalator extends Widget implements RequiresResize, private final class SpacerImpl implements Spacer { private TableCellElement spacerElement; private TableRowElement root; + private DivElement deco; private int rowIndex; private double height = -1; private boolean domHasBeenSetup = false; + private double decoHeight; public SpacerImpl(int rowIndex) { this.rowIndex = rowIndex; @@ -4613,6 +4621,7 @@ public class Escalator extends Widget implements RequiresResize, spacerElement = TableCellElement.as(DOM.createTD()); root.appendChild(spacerElement); root.setPropertyInt(SPACER_LOGICAL_ROW_PROPERTY, rowIndex); + deco = DivElement.as(DOM.createDiv()); } public void setPositionDiff(double x, double y) { @@ -4637,12 +4646,19 @@ public class Escalator extends Widget implements RequiresResize, return root; } + @Override + public Element getDecoElement() { + return deco; + } + public void setPosition(double x, double y) { positions.set(getRootElement(), x, y); + positions.set(getDecoElement(), 0, y); } public void setStylePrimaryName(String style) { UIObject.setStylePrimaryName(root, style + "-spacer"); + UIObject.setStylePrimaryName(deco, style + "-spacer-deco"); } public void setHeight(double height) { @@ -4731,8 +4747,42 @@ public class Escalator extends Widget implements RequiresResize, verticalScrollbar.setScrollSize(verticalScrollbar .getScrollSize() + heightDiff); } + + updateDecoratorGeometry(height); + } + + /** Resizes and places the decorator. */ + private void updateDecoratorGeometry(double detailsHeight) { + Style style = deco.getStyle(); + decoHeight = detailsHeight + getBody().getDefaultRowHeight(); + + style.setTop( + -(getBody().getDefaultRowHeight() - getBorderTopHeight(getElement())), + Unit.PX); + style.setHeight(decoHeight, Unit.PX); } + private native double getBorderTopHeight(Element spacerCell) + /*-{ + if (typeof $wnd.getComputedStyle === 'function') { + var computedStyle = $wnd.getComputedStyle(spacerCell); + var borderTopWidth = computedStyle['borderTopWidth']; + var width = parseFloat(borderTopWidth); + return width; + } else { + var spacerRow = spacerCell.offsetParent; + var cloneCell = spacerCell.cloneNode(false); + spacerRow.appendChild(cloneCell); + cloneCell.style.height = "10px"; // IE8 wants the height to be set to something... + var heightWithBorder = cloneCell.offsetHeight; + cloneCell.style.borderTopWidth = "0"; + var heightWithoutBorder = cloneCell.offsetHeight; + spacerRow.removeChild(cloneCell); + + return heightWithBorder - heightWithoutBorder; + } + }-*/; + @Override public Element getElement() { return spacerElement; @@ -4795,6 +4845,35 @@ public class Escalator extends Widget implements RequiresResize, public void hide() { getRootElement().getStyle().setDisplay(Display.NONE); } + + /** + * Crop the decorator element so that it doesn't overlap the header + * and footer sections. + * + * @param bodyTop + * the top cordinate of the escalator body + * @param bodyBottom + * the bottom cordinate of the escalator body + * @param decoWidth + * width of the deco + */ + private void updateDecoClip(final double bodyTop, + final double bodyBottom, final double decoWidth) { + final int top = deco.getAbsoluteTop(); + final int bottom = deco.getAbsoluteBottom(); + if (top < bodyTop || bottom > bodyBottom) { + final double topClip = Math.max(0.0D, bodyTop - top); + final double bottomClip = decoHeight + - Math.max(0.0D, bottom - bodyBottom); + final String clip = new StringBuilder("rect(") + .append(topClip).append("px,").append(decoWidth) + .append("px,").append(bottomClip).append("px,0") + .toString(); + deco.getStyle().setProperty("clip", clip); + } else { + deco.getStyle().clearProperty("clip"); + } + } } private final TreeMap rowIndexToSpacer = new TreeMap(); @@ -4818,6 +4897,9 @@ public class Escalator extends Widget implements RequiresResize, }; private HandlerRegistration spacerScrollerRegistration; + /** Width of the spacers' decos. Calculated once then cached. */ + private double spacerDecoWidth = 0.0D; + public void setSpacer(int rowIndex, double height) throws IllegalArgumentException { @@ -4883,9 +4965,10 @@ public class Escalator extends Widget implements RequiresResize, } public void reapplySpacerWidths() { + // FIXME #16266 , spacers get couple pixels too much because borders + final double width = getInnerWidth() - spacerDecoWidth; for (SpacerImpl spacer : rowIndexToSpacer.values()) { - spacer.getRootElement().getStyle() - .setWidth(getInnerWidth(), Unit.PX); + spacer.getRootElement().getStyle().setWidth(width, Unit.PX); } } @@ -4916,6 +4999,7 @@ public class Escalator extends Widget implements RequiresResize, destroySpacerContent(spacer); spacer.setHeight(0); // resets row offsets spacer.getRootElement().removeFromParent(); + spacer.getDecoElement().removeFromParent(); } removedSpacers.clear(); @@ -5218,6 +5302,15 @@ public class Escalator extends Widget implements RequiresResize, body.getElement().appendChild(spacerRoot); spacer.setupDom(height); + spacerDecoContainer.appendChild(spacer.getDecoElement()); + if (spacerDecoContainer.getParentElement() == null) { + getElement().appendChild(spacerDecoContainer); + // calculate the spacer deco width, it won't change + spacerDecoWidth = WidgetUtil + .getRequiredWidthBoundingClientRectDouble(spacer + .getDecoElement()); + } + initSpacerContent(spacer); body.sortDomElements(); @@ -5340,6 +5433,22 @@ public class Escalator extends Widget implements RequiresResize, spacer.setRowIndex(spacer.getRow() + numberOfRows); } } + + private void updateSpacerDecosVisibility() { + final Range visibleRowRange = getVisibleRowRange(); + Collection visibleSpacers = rowIndexToSpacer.subMap( + visibleRowRange.getStart() - 1, + visibleRowRange.getEnd() + 1).values(); + if (!visibleSpacers.isEmpty()) { + final double top = tableWrapper.getAbsoluteTop() + + header.getHeightOfSection(); + final double bottom = tableWrapper.getAbsoluteBottom() + - footer.getHeightOfSection(); + for (SpacerImpl spacer : visibleSpacers) { + spacer.updateDecoClip(top, bottom, spacerDecoWidth); + } + } + } } private class ElementPositionBookkeeper { @@ -5495,6 +5604,8 @@ public class Escalator extends Widget implements RequiresResize, .createDiv()); private final DivElement headerDeco = DivElement.as(DOM.createDiv()); private final DivElement footerDeco = DivElement.as(DOM.createDiv()); + private final DivElement spacerDecoContainer = DivElement.as(DOM + .createDiv()); private PositionFunction position; @@ -5570,6 +5681,8 @@ public class Escalator extends Widget implements RequiresResize, setStylePrimaryName("v-escalator"); + spacerDecoContainer.setAttribute("aria-hidden", "true"); + // init default dimensions setHeight(null); setWidth(null); @@ -6294,6 +6407,8 @@ public class Escalator extends Widget implements RequiresResize, UIObject.setStylePrimaryName(footerDeco, style + "-footer-deco"); UIObject.setStylePrimaryName(horizontalScrollbarDeco, style + "-horizontal-scrollbar-deco"); + UIObject.setStylePrimaryName(spacerDecoContainer, style + + "-spacer-deco-container"); header.setStylePrimaryName(style); body.setStylePrimaryName(style); diff --git a/client/src/com/vaadin/client/widgets/Grid.java b/client/src/com/vaadin/client/widgets/Grid.java index 77bddb281f..f45d8ef3b4 100644 --- a/client/src/com/vaadin/client/widgets/Grid.java +++ b/client/src/com/vaadin/client/widgets/Grid.java @@ -2864,21 +2864,13 @@ public class Grid extends ResizeComposite implements private class GridSpacerUpdater implements SpacerUpdater { - private static final String DECO_CLASSNAME = "deco"; - private static final String CONTENT_CLASSNAME = "content"; private static final String STRIPE_CLASSNAME = "stripe"; private final Map elementToWidgetMap = new HashMap(); @Override public void init(Spacer spacer) { - initStructure(spacer); - Element root = getDetailsRoot(spacer); - - assert root.getFirstChild() == null : "The spacer's" - + " element should be empty at this point. (row: " - + spacer.getRow() + ", child: " + root.getFirstChild() - + ")"; + initTheming(spacer); int rowIndex = spacer.getRow(); @@ -2893,12 +2885,13 @@ public class Grid extends ResizeComposite implements } final double spacerHeight; + Element spacerElement = spacer.getElement(); if (detailsWidget == null) { - root.removeAllChildren(); + spacerElement.removeAllChildren(); spacerHeight = DETAILS_ROW_INITIAL_HEIGHT; } else { Element element = detailsWidget.getElement(); - root.appendChild(element); + spacerElement.appendChild(element); setParent(detailsWidget, Grid.this); Widget previousWidget = elementToWidgetMap.put(element, detailsWidget); @@ -2911,25 +2904,24 @@ public class Grid extends ResizeComposite implements * re-measure it to make sure that it's the correct height. */ double measuredHeight = WidgetUtil - .getRequiredHeightBoundingClientRectDouble(root); - assert getElement().isOrHasChild(root) : "The spacer element wasn't in the DOM during measurement, but was assumed to be."; + .getRequiredHeightBoundingClientRectDouble(spacerElement); + assert getElement().isOrHasChild(spacerElement) : "The spacer element wasn't in the DOM during measurement, but was assumed to be."; spacerHeight = measuredHeight; } escalator.getBody().setSpacer(rowIndex, spacerHeight); - updateDecoratorGeometry(spacerHeight, spacer); } @Override public void destroy(Spacer spacer) { - Element root = getDetailsRoot(spacer); + Element spacerElement = spacer.getElement(); - assert getElement().isOrHasChild(root) : "Trying " + assert getElement().isOrHasChild(spacerElement) : "Trying " + "to destroy a spacer that is not connected to this " + "Grid's DOM. (row: " + spacer.getRow() + ", element: " - + root + ")"; + + spacerElement + ")"; - Widget detailsWidget = elementToWidgetMap.remove(root + Widget detailsWidget = elementToWidgetMap.remove(spacerElement .getFirstChildElement()); if (detailsWidget != null) { @@ -2938,107 +2930,27 @@ public class Grid extends ResizeComposite implements * returned a null widget. */ - assert root.getFirstChild() != null : "The " + assert spacerElement.getFirstChild() != null : "The " + "details row to destroy did not contain a widget - " + "probably removed by something else without " + "permission? (row: " + spacer.getRow() - + ", element: " + root + ")"; + + ", element: " + spacerElement + ")"; setParent(detailsWidget, null); - root.removeAllChildren(); + spacerElement.removeAllChildren(); } } - /** - * Initializes the spacer element into a details structure, containing a - * decorator and a slot for the details widget. - */ - private void initStructure(Spacer spacer) { + private void initTheming(Spacer spacer) { Element spacerRoot = spacer.getElement(); - if (spacerRoot.getChildCount() == 0) { - Element deco = DOM.createDiv(); - deco.setClassName(DECO_CLASSNAME); - - Element detailsContent = DOM.createDiv(); - detailsContent.setClassName(CONTENT_CLASSNAME); - - if (spacer.getRow() % 2 == 1) { - spacerRoot.getParentElement() - .addClassName(STRIPE_CLASSNAME); - } - - spacerRoot.appendChild(deco); - spacerRoot.appendChild(detailsContent); - } - - else { - if (spacer.getRow() % 2 == 1) { - spacerRoot.getParentElement() - .addClassName(STRIPE_CLASSNAME); - } else { - spacerRoot.getParentElement().removeClassName( - STRIPE_CLASSNAME); - } - - /* - * The only case when we get into this else branch is when the - * previous generated details element was a null Widget. In - * those situations, we don't call destroy on the content, but - * simply reuse it as-is. - */ - assert getDetailsRoot(spacer).getChildCount() == 0 : "This " - + "code should never be triggered unless the details " - + "root already was empty"; + if (spacer.getRow() % 2 == 1) { + spacerRoot.getParentElement().addClassName(STRIPE_CLASSNAME); + } else { + spacerRoot.getParentElement().removeClassName(STRIPE_CLASSNAME); } } - /** Gets the decorator element from the DOM structure. */ - private Element getDecorator(Spacer spacer) { - TableCellElement td = TableCellElement.as(spacer.getElement()); - Element decorator = td.getFirstChildElement(); - return decorator; - } - - /** Gets the element for the details widget from the DOM structure. */ - private Element getDetailsRoot(Spacer spacer) { - Element detailsRoot = getDecorator(spacer).getNextSiblingElement(); - return detailsRoot; - } - - /** Resizes and places the decorator. */ - private void updateDecoratorGeometry(double detailsHeight, Spacer spacer) { - Element decorator = getDecorator(spacer); - Style style = decorator.getStyle(); - double rowHeight = escalator.getBody().getDefaultRowHeight(); - double borderHeight = getBorderHeight(spacer); - - style.setTop(-(rowHeight - borderHeight), Unit.PX); - style.setHeight(detailsHeight + rowHeight, Unit.PX); - } - - private native double getBorderHeight(Spacer spacer) - /*-{ - var spacerCell = spacer.@com.vaadin.client.widget.escalator.Spacer::getElement()(); - if (typeof $wnd.getComputedStyle === 'function') { - var computedStyle = $wnd.getComputedStyle(spacerCell); - var borderTopWidth = computedStyle['borderTopWidth']; - var width = parseFloat(borderTopWidth); - return width; - } else { - var spacerRow = spacerCell.offsetParent; - var cloneCell = spacerCell.cloneNode(false); - spacerRow.appendChild(cloneCell); - cloneCell.style.height = "10px"; // IE8 wants the height to be set to something... - var heightWithBorder = cloneCell.offsetHeight; - cloneCell.style.borderTopWidth = "0"; - var heightWithoutBorder = cloneCell.offsetHeight; - spacerRow.removeChild(cloneCell); - - console.log(heightWithBorder+" - "+heightWithoutBorder); - return heightWithBorder - heightWithoutBorder; - } - }-*/; } /** @@ -6539,18 +6451,14 @@ public class Grid extends ResizeComposite implements public com.google.gwt.user.client.Element getSubPartElement(String subPart) { /* - * gandles details[] (translated to spacer[] for Escalator), cell[], + * handles details[] (translated to spacer[] for Escalator), cell[], * header[] and footer[] */ Element escalatorElement = escalator.getSubPartElement(subPart .replaceFirst("^details\\[", "spacer[")); if (escalatorElement != null) { - if (subPart.startsWith("details[")) { - return DOM.asOld(parseDetails(escalatorElement)); - } else { - return DOM.asOld(escalatorElement); - } + return DOM.asOld(escalatorElement); } SubPartArguments args = Escalator.parseSubPartArguments(subPart); @@ -6563,23 +6471,6 @@ public class Grid extends ResizeComposite implements return null; } - @SuppressWarnings("static-method") - private Element parseDetails(Element spacer) { - assert spacer.getChildCount() == 2 : "Unexpected structure for details "; - - Element decorator = spacer.getFirstChildElement(); - assert decorator != null : "unexpected spacer DOM structure"; - assert decorator.getClassName() - .equals(GridSpacerUpdater.DECO_CLASSNAME) : "unexpected first details element"; - - Element spacerRoot = decorator.getNextSiblingElement(); - assert spacerRoot != null : "unexpected spacer DOM structure"; - assert spacerRoot.getClassName().equals( - GridSpacerUpdater.CONTENT_CLASSNAME) : "unexpected second details element"; - - return DOM.asOld(spacerRoot); - } - private Element getSubPartElementEditor(SubPartArguments args) { if (!args.getType().equalsIgnoreCase("editor") -- 2.39.5