Set display:none in addition to hidden attribute when hiding elem…

…ents inside a shadow root (#9026)

Set `display:none` in addition to `hidden` attribute when hiding elements inside a shadow root

Fixes #8256

flow-client/src/main/java/com/vaadin/client/PolymerUtils.java

@@ -629,4 +629,20 @@ public static native void setProperty(Element element, String path,
          element.set(path, value);
      }-*/;
 
+    /**
+     * Returns true if and only if the element has a shadow root ancestor.
+     * 
+     * @param element
+     *            the element to test
+     * @return whether the element is in a shadow root
+     */
+    public static native boolean isInShadowRoot(Element element)
+    /*-{
+        while (element.parentNode && (element = element.parentNode)) {
+            if (element.toString() === '[object ShadowRoot]') {
+                return true;
+            }
+        }
+        return false;
+     }-*/;
 }

flow-client/src/main/java/com/vaadin/client/flow/binding/SimpleElementBindingStrategy.java

@@ -625,39 +625,55 @@ private void updateVisibility(JsArray<EventRemover> listeners,
                     .setValue(true);
             restoreInitialHiddenAttribute(element, visibilityData);
         } else {
-            updateVisibility(element, visibilityData, Boolean.TRUE);
+            setElementInvisible(element, visibilityData);
         }
     }
 
-    private void updateVisibility(Element element, NodeMap visibilityData,
-            Boolean visibility) {
+    private void setElementInvisible(Element element, NodeMap visibilityData) {
         storeInitialHiddenAttribute(element, visibilityData);
         updateAttributeValue(
                 visibilityData.getNode().getTree().getRegistry()
                         .getApplicationConfiguration(),
-                element, HIDDEN_ATTRIBUTE, visibility);
+                element, HIDDEN_ATTRIBUTE, Boolean.TRUE);
+        if (PolymerUtils.isInShadowRoot(element)) {
+            element.getStyle().setDisplay("none");
+        }
     }
 
     private void restoreInitialHiddenAttribute(Element element,
             NodeMap visibilityData) {
-        MapProperty initialVisibility = storeInitialHiddenAttribute(element,
-                visibilityData);
+        storeInitialHiddenAttribute(element, visibilityData);
+        MapProperty initialVisibility = visibilityData
+                .getProperty(NodeProperties.VISIBILITY_HIDDEN_PROPERTY);
         if (initialVisibility.hasValue()) {
             updateAttributeValue(
                     visibilityData.getNode().getTree().getRegistry()
                             .getApplicationConfiguration(),
                     element, HIDDEN_ATTRIBUTE, initialVisibility.getValue());
         }
+
+        MapProperty initialDisplay = visibilityData
+                .getProperty(NodeProperties.VISIBILITY_STYLE_DISPLAY_PROPERTY);
+        if (initialDisplay.hasValue()) {
+            final String initialValue = initialDisplay.getValue().toString();
+            element.getStyle().setDisplay(initialValue);
+        }
     }
 
-    private MapProperty storeInitialHiddenAttribute(Element element,
+    private void storeInitialHiddenAttribute(Element element,
             NodeMap visibilityData) {
         MapProperty initialVisibility = visibilityData
                 .getProperty(NodeProperties.VISIBILITY_HIDDEN_PROPERTY);
         if (!initialVisibility.hasValue()) {
             initialVisibility.setValue(element.getAttribute(HIDDEN_ATTRIBUTE));
         }
-        return initialVisibility;
+
+        MapProperty initialDisplay = visibilityData
+                .getProperty(NodeProperties.VISIBILITY_STYLE_DISPLAY_PROPERTY);
+        if (PolymerUtils.isInShadowRoot(element) && !initialDisplay.hasValue()
+                && element.getStyle() != null) {
+            initialDisplay.setValue(element.getStyle().getDisplay());
+        }
     }
 
     private void doBind(StateNode node, BinderContext nodeFactory) {

flow-client/src/test-gwt/java/com/vaadin/client/flow/GwtBasicElementBinderTest.java

@@ -1483,6 +1483,7 @@ public void testBindInvisibleNode() {
         Binder.bind(node, element);
 
         assertEquals(Boolean.TRUE.toString(), element.getAttribute("hidden"));
+        assertEquals("", element.getStyle().getDisplay());
     }
 
     public void testBindVisibleNode() {
@@ -1580,6 +1581,7 @@ public void testBindHiddenElement_stateNodeIsVisible_elementStaysHidden() {
         Binder.bind(node, element);
 
         assertEquals(Boolean.TRUE.toString(), element.getAttribute("hidden"));
+        assertEquals("", element.getStyle().getDisplay());
     }
 
     /**
@@ -1605,6 +1607,7 @@ public void testBindHiddenElement_stateNodeChangesVisibility_elementStaysHidden(
         Reactive.flush();
 
         assertEquals(Boolean.TRUE.toString(), element.getAttribute("hidden"));
+        assertEquals("", element.getStyle().getDisplay());
 
         element.setAttribute("hidden", null);
 
@@ -1613,8 +1616,51 @@ public void testBindHiddenElement_stateNodeChangesVisibility_elementStaysHidden(
         Reactive.flush();
 
         assertEquals(Boolean.TRUE.toString(), element.getAttribute("hidden"));
+        assertEquals("", element.getStyle().getDisplay());
     }
 
+
+    /**
+     * The element is in shadowroot and state node is hidden. The element gets
+     * attribute "hidden" and "display: none".
+     */
+    public void testBindHiddenElement_elementInShadowRoot_elementHasDisplayNone() {
+        addShadowRootElement(element);
+        setTag();
+        node.setDomNode(element);
+        Binder.bind(node, element);
+        setVisible(false);
+        Reactive.flush();
+        assertEquals(Boolean.TRUE.toString(), element.getAttribute("hidden"));
+        assertEquals("none", element.getStyle().getDisplay());
+    }
+
+    /**
+     * The element is in shadowroot and has a custom display property. When
+     * hidden gets attribute "hidden" and "display:none". When unhidden
+     * "display" property is restored.
+     */
+    public void testBindHiddenElement_elementInShadowRootAndHasInitialDisplayProperty_displayPropertyRestoredWhenUnhidden() {
+        addShadowRootElement(element);
+        setTag();
+        node.setDomNode(element);
+        element.getStyle().setDisplay("inline-block");
+
+        Binder.bind(node, element);
+        setVisible(false);
+        Reactive.flush();
+
+        assertEquals(Boolean.TRUE.toString(), element.getAttribute("hidden"));
+        assertEquals("none", element.getStyle().getDisplay());
+
+        setVisible(true);
+        Reactive.flush();
+
+        assertNull(element.getAttribute("hidden"));
+        assertEquals("inline-block", element.getStyle().getDisplay());
+    }
+
+
     /**
      * The StateNode is visible (the visibility is true).
      *
@@ -1635,12 +1681,14 @@ public void testBindNotHiddenElement_stateNodeChangesVisibility_elementIsNotHidd
         Reactive.flush();
 
         assertEquals(Boolean.TRUE.toString(), element.getAttribute("hidden"));
+        assertEquals("", element.getStyle().getDisplay());
 
         setVisible(true);
 
         Reactive.flush();
 
         assertNull(element.getAttribute("hidden"));
+        assertEquals("", element.getStyle().getDisplay());
     }
 
     /**
@@ -1702,12 +1750,14 @@ public void testBindHiddenElement_stateNodeIsInvisible_elementStaysHidden() {
 
         Reactive.flush();
         assertEquals(Boolean.TRUE.toString(), element.getAttribute("hidden"));
+        assertEquals("", element.getStyle().getDisplay());
 
         setVisible(true);
 
         Reactive.flush();
 
         assertEquals(Boolean.TRUE.toString(), element.getAttribute("hidden"));
+        assertEquals("", element.getStyle().getDisplay());
     }
 
     public void testSimpleElementBindingStrategy_regularElement_needsBind() {
@@ -1844,6 +1894,10 @@ private native Element addShadowRootElement(Element element)
         var shadowRoot = $doc.createElement("div");
         element.shadowRoot = shadowRoot;
         element.root = shadowRoot;
+        shadowRoot.toString = function() {
+            return '[object ShadowRoot]';
+        }
+        element.parentNode = shadowRoot;
         return shadowRoot;
     }-*/;
 

flow-server/src/main/java/com/vaadin/flow/internal/nodefeature/NodeProperties.java

@@ -87,6 +87,15 @@ public final class NodeProperties {
      */
     public static final String VISIBILITY_HIDDEN_PROPERTY = "hidden";
 
+    /**
+     * The property used on the client side only in addition to
+     * {@link #VISIBLE}. It stores the client side value of the CSS "display"
+     * property to be able to restore when making a hidden element visible
+     * again. Used only when the element is inside a shadow root, and the CSS
+     * "display: none" is set in addition the "hidden" attribute.
+     */
+    public static final String VISIBILITY_STYLE_DISPLAY_PROPERTY = "styleDisplay";
+
     /**
      * The property in Json object which marks the object as special value
      * transmitting URI (not just any string).

flow-tests/test-root-context/frontend/template-inner.js

@@ -0,0 +1,15 @@
+import {PolymerElement} from '@polymer/polymer/polymer-element.js';
+import {html} from '@polymer/polymer/lib/utils/html-tag.js';
+
+class TemplateInner extends PolymerElement {
+
+  static get is() { return 'template-inner' }
+
+  static get template() {
+    return html`
+        <div>Hello template inner</div>
+    `;
+  }
+}
+
+customElements.define(TemplateInner.is, TemplateInner);

flow-tests/test-root-context/frontend/template-outer.js

@@ -0,0 +1,17 @@
+import {PolymerElement} from '@polymer/polymer/polymer-element.js';
+import {html} from '@polymer/polymer/lib/utils/html-tag.js';
+import './template-inner.js';
+
+class TemplateOuter extends PolymerElement {
+
+  static get is() { return 'template-outer'; }
+
+  static get template() {
+    return html`
+            <div>Hello template outer</div>
+            <template-inner id="inner" style="display: block"></template-inner>
+    `;
+  }
+}
+
+customElements.define(TemplateOuter.is, TemplateOuter);

flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/template/InnerTemplateVisibilityView.java

@@ -0,0 +1,62 @@
+package com.vaadin.flow.uitest.ui.template;
+
+import com.vaadin.flow.component.Tag;
+import com.vaadin.flow.component.dependency.HtmlImport;
+import com.vaadin.flow.component.dependency.JsModule;
+import com.vaadin.flow.component.html.NativeButton;
+import com.vaadin.flow.component.polymertemplate.Id;
+import com.vaadin.flow.component.polymertemplate.PolymerTemplate;
+import com.vaadin.flow.router.Route;
+import com.vaadin.flow.templatemodel.TemplateModel;
+import com.vaadin.flow.uitest.servlet.ViewTestLayout;
+import com.vaadin.flow.uitest.ui.AbstractDivView;
+
+@Route(value = "com.vaadin.flow.uitest.ui.template.InnerTemplateVisibilityView", layout = ViewTestLayout.class)
+public class InnerTemplateVisibilityView extends AbstractDivView {
+
+    public static final String TOGGLE_INNER_VISIBILITY_BUTTON_ID = "toggleInnerVisibility";
+    public static final String TOGGLE_OUTER_VISIBILITY_BUTTON_ID = "toggleOuterVisibility";
+    public static final String INNER_ID = "inner";
+    public static final String OUTER_ID = "outer";
+
+    @Tag("template-inner")
+    @JsModule("./template-inner.js")
+    @HtmlImport("frontend://com/vaadin/flow/uitest/ui/template/template-inner.html")
+    public static class Inner extends PolymerTemplate<Inner.Model> {
+        public static class Model implements TemplateModel {}
+
+        public Inner() {
+        }
+    }
+
+    @Tag("template-outer")
+    @JsModule("./template-outer.js")
+    @HtmlImport("frontend://com/vaadin/flow/uitest/ui/template/template-outer.html")
+    public static class Outer extends PolymerTemplate<Outer.Model> {
+        public static class Model implements TemplateModel {}
+
+        @Id("inner")
+        Inner inner;
+
+        public Outer() {
+        }
+    }
+
+    public InnerTemplateVisibilityView() {
+        Outer outer = new Outer();
+        outer.setId(OUTER_ID);
+        outer.inner.setId(INNER_ID);
+
+        NativeButton toggleOuterVisibilityButton = new NativeButton(
+                "Toggle visibility of outer",
+                e -> outer.setVisible(!outer.isVisible()));
+        toggleOuterVisibilityButton.setId(TOGGLE_OUTER_VISIBILITY_BUTTON_ID);
+
+        NativeButton toggleInnerVisibility = new NativeButton(
+                "Toggle visibility of inner",
+                e -> outer.inner.setVisible(!outer.inner.isVisible()));
+        toggleInnerVisibility.setId(TOGGLE_INNER_VISIBILITY_BUTTON_ID);
+
+        add(toggleOuterVisibilityButton, toggleInnerVisibility, outer);
+    }
+}

flow-tests/test-root-context/src/main/webapp/frontend/com/vaadin/flow/uitest/ui/template/template-inner.html

@@ -0,0 +1,15 @@
+<link rel="import" href="/frontend/bower_components/polymer/polymer-element.html">
+
+<dom-module id="template-inner">
+    <template>
+        <div>Hello template inner</div>
+    </template>
+
+    <script>
+      class TemplateInner extends Polymer.Element {
+        static get is() { return 'template-inner' }
+      }
+
+      customElements.define(TemplateInner.is, TemplateInner);
+    </script>
+</dom-module>

flow-tests/test-root-context/src/main/webapp/frontend/com/vaadin/flow/uitest/ui/template/template-outer.html

@@ -0,0 +1,16 @@
+<link rel="import" href="/frontend/bower_components/polymer/polymer-element.html">
+
+<dom-module id="template-outer">
+    <template>
+        <div>Hello template outer</div>
+        <template-inner id="inner" style="display: block"></template-inner>
+    </template>
+
+    <script>
+      class TemplateOuter extends Polymer.Element {
+        static get is() { return 'template-outer'; }
+      }
+
+      customElements.define(TemplateOuter.is, TemplateOuter);
+    </script>
+</dom-module>