-
Notifications
You must be signed in to change notification settings - Fork 161
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Set display:none
in addition to hidden
attribute when hiding elements inside a shadow root
#9026
Merged
denis-anisimov
merged 8 commits into
master
from
joheriks/8256-template-visibility-in-shadow-root
Sep 22, 2020
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
bca4483
Add `visibility: none` in additional to `hidden` attribute for invisi…
b8a3922
Integration test
b55fe75
Only set 'display: none' if hidden element is inside a shadow root
424826c
Check for display CSS is not added in existing GWT tests that assert …
00559bf
Minor fixes based on review
97bf38f
Corrected test comment
7099d79
Comment fix
ec80684
Added two unit tests
File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
12 changes: 12 additions & 0 deletions
12
flow-tests/test-root-context/frontend/lit/lit-template-inner.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
import {html, LitElement} from 'lit-element'; | ||
|
||
class TemplateInner extends LitElement { | ||
|
||
render() { | ||
return html` | ||
<div>Hello template inner</div> | ||
`; | ||
} | ||
} | ||
|
||
customElements.define('lit-template-inner', TemplateInner); |
14 changes: 14 additions & 0 deletions
14
flow-tests/test-root-context/frontend/lit/lit-template-outer.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
import {html, LitElement} from 'lit-element'; | ||
import './lit-template-inner.js'; | ||
|
||
class TemplateOuter extends LitElement { | ||
|
||
render() { | ||
return html` | ||
<div>Hello template outer</div> | ||
<lit-template-inner id="inner" style="display: block"></lit-template-inner> | ||
`; | ||
} | ||
} | ||
|
||
customElements.define('lit-template-outer', TemplateOuter); |
54 changes: 54 additions & 0 deletions
54
...text/src/main/java/com/vaadin/flow/uitest/ui/littemplate/InnerTemplateVisibilityView.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
package com.vaadin.flow.uitest.ui.littemplate; | ||
|
||
import com.vaadin.flow.component.Tag; | ||
import com.vaadin.flow.component.dependency.JsModule; | ||
import com.vaadin.flow.component.html.NativeButton; | ||
import com.vaadin.flow.component.littemplate.LitTemplate; | ||
import com.vaadin.flow.component.polymertemplate.Id; | ||
import com.vaadin.flow.router.Route; | ||
import com.vaadin.flow.uitest.servlet.ViewTestLayout; | ||
import com.vaadin.flow.uitest.ui.AbstractDivView; | ||
|
||
@Route(value = "com.vaadin.flow.uitest.ui.littemplate.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("lit-template-inner") | ||
@JsModule("./lit/lit-template-inner.js") | ||
public static class Inner extends LitTemplate { | ||
public Inner() { | ||
} | ||
} | ||
|
||
@Tag("lit-template-outer") | ||
@JsModule("./lit/lit-template-outer.js") | ||
public static class Outer extends LitTemplate { | ||
@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); | ||
} | ||
} |
77 changes: 77 additions & 0 deletions
77
...ontext/src/test/java/com/vaadin/flow/uitest/ui/littemplate/InnerTemplateVisibilityIT.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,77 @@ | ||
package com.vaadin.flow.uitest.ui.littemplate; | ||
|
||
import org.junit.Assert; | ||
import org.junit.Test; | ||
import org.openqa.selenium.By; | ||
import org.openqa.selenium.WebElement; | ||
|
||
import com.vaadin.flow.component.html.testbench.NativeButtonElement; | ||
import com.vaadin.flow.testutil.ChromeBrowserTest; | ||
|
||
public class InnerTemplateVisibilityIT extends ChromeBrowserTest { | ||
|
||
@Test | ||
public void innerTemplateIsHiddenWithDisplayNone() { | ||
open(); | ||
|
||
// when inner is hidden | ||
NativeButtonElement toggleButton = $(NativeButtonElement.class) | ||
.id(InnerTemplateVisibilityView.TOGGLE_INNER_VISIBILITY_BUTTON_ID); | ||
toggleButton.click(); | ||
|
||
// then: element is not visible, attribute 'hidden' and 'display: none' | ||
// set | ||
WebElement outer = findElement( | ||
By.id(InnerTemplateVisibilityView.OUTER_ID)); | ||
WebElement inner = findInShadowRoot(outer, | ||
By.id(InnerTemplateVisibilityView.INNER_ID)).get(0); | ||
Assert.assertFalse("expected inner to be hidden", inner.isDisplayed()); | ||
Assert.assertNotNull("expected attribute hidden on inner", | ||
inner.getAttribute("hidden")); | ||
Assert.assertEquals("expected 'display: none' on inner", "none", | ||
inner.getCssValue("display")); | ||
} | ||
|
||
@Test | ||
public void innerTemplateDisplayStyleRestored() { | ||
open(); | ||
|
||
// when inner is hidden and unhidden | ||
NativeButtonElement toggleButton = $(NativeButtonElement.class) | ||
.id(InnerTemplateVisibilityView.TOGGLE_INNER_VISIBILITY_BUTTON_ID); | ||
toggleButton.click(); | ||
toggleButton.click(); | ||
|
||
// then: element is visible, attribute and 'display: none' are no longer | ||
// present | ||
WebElement outer = findElement( | ||
By.id(InnerTemplateVisibilityView.OUTER_ID)); | ||
WebElement inner = findInShadowRoot(outer, | ||
By.id(InnerTemplateVisibilityView.INNER_ID)).get(0); | ||
Assert.assertTrue("expected inner to be visible", inner.isDisplayed()); | ||
Assert.assertNull("inner should not have attribute hidden", | ||
inner.getAttribute("hidden")); | ||
Assert.assertEquals("expected 'display: block' on inner", "block", | ||
inner.getCssValue("display")); | ||
} | ||
|
||
@Test | ||
public void outerTemplateIsHiddenWithAttributeOnly() { | ||
open(); | ||
|
||
// when hidden | ||
NativeButtonElement toggleButton = $(NativeButtonElement.class) | ||
.id(InnerTemplateVisibilityView.TOGGLE_OUTER_VISIBILITY_BUTTON_ID); | ||
toggleButton.click(); | ||
|
||
// then: element is not visible, attribute 'hidden' is set but | ||
// 'display: none' is not set | ||
WebElement outer = findElement( | ||
By.id(InnerTemplateVisibilityView.OUTER_ID)); | ||
Assert.assertFalse("expected outer to be hidden", outer.isDisplayed()); | ||
Assert.assertNotNull("expected attribute hidden on outer", | ||
outer.getAttribute("hidden")); | ||
Assert.assertEquals("expected no style attribute", "", | ||
outer.getAttribute("style")); | ||
} | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need more GWT tests for this.
It's possible to emulate code with shadow root with this code or change one.
You may create Elements hierarchy so that one element in the hierarchy returns '[object ShadowRoot]'.
Just fake it: create a fake
toString
method and return this string from it.Another way could be reimplement this method a bit so that it doesn't use
toString
but check whether an element has a parent whoseshadowRoot
property is set to the element.Then it's also possible to fake
shadowRoot
.We have quite many tests like that which fakes elements.
See e.g.
GwtBasicElementBinderTest
:initPolymer
,addShadowRootElement
, etc.With mocking/faking it should be possible to write the same tests for
display
style as we already have forhidden
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, thanks for the hints. Added two unit tests.