Skip to content
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

dom-if that contains a slot not working (without restamp) #4977

Closed
simonfuhrmann opened this issue Dec 6, 2017 · 3 comments
Closed

dom-if that contains a slot not working (without restamp) #4977

simonfuhrmann opened this issue Dec 6, 2017 · 3 comments

Comments

@simonfuhrmann
Copy link
Contributor

simonfuhrmann commented Dec 6, 2017

Description

The following code is not correctly working:

<template is="dom-if" if="[[toggle]]">
  <slot></slot>
</template>

When the toggle becomes true and the template is stamped for the first time, the slotted content appears. When the toggle becomes false again, the slotted content does not disappear. When the restamp attribute is applied to dom-if, it works. But without restamp, it doesn't.

Live Demo

http://jsbin.com/fazewarake/edit?html,console,output

Steps to Reproduce

  1. Create test-slotted element
  2. Create test-main element and put <test-slotted> in the content
  3. Create a dom-if in test-main with a toggle that toggles a <slot>
  4. Observe how the slotted element does not toggle

Expected Results

Toggling the slotted element using dom-if works.

Actual Results

The slotted element appears, but then doesn't disappear when the dom-if becomes false. Note that when applying the restamp attribute to dom-if, it works.

Versions

  • Polymer: v2.3.0
  • webcomponents: v1.0.20
@simonfuhrmann simonfuhrmann changed the title dom-if that contains slots not working (without restamp) dom-if that contains a slot not working (without restamp) Dec 6, 2017
@kevinpschaaf
Copy link
Member

kevinpschaaf commented Dec 6, 2017

In non-restamp mode, dom-if simply sets display: none; to all top-level children stamped from the template. If one of these is a <slot>, only the slot gets display: none. However, this does not have the effect of actually hiding the assigned children in either native implementation (Chrome & Safari), as the implementations don't currently respect the display value of the slot, but rather implement the spec'ed display: contents behavior of slot as a special case (see this thread for background).

It's not clear if implementations will actually change to use actual display: contents; once that CSS feature ships. If so, the native implementations could actually work correctly with the existing code. However, the shadycss polyfill will likely never be able to support that feature without real cross-platform display: contents implementations, as it would be prohibitively expensive.

As for alternatives, dom-if could in theory recurse into assignedNodes of top-level slots and display: none those, but there would be a high risk of getting out of sync when nodes become unslotted while hidden, etc.

As such, I think the best workaround for now would be to use restamp mode for slots. We should update the documentation to make this clear. We might be able to warn as well, since we already track whether a template has a slot for polyfill efficiency.

@kevinpschaaf kevinpschaaf self-assigned this Dec 6, 2017
@sorvell
Copy link
Contributor

sorvell commented Dec 12, 2017

Alternatively, simply wrapping the <slot> in another element like a <span> can also address the issue: http://jsbin.com/fadiyukafi/1/edit?html,console,output

@sorvell
Copy link
Contributor

sorvell commented Dec 12, 2017

It may be possible for us to fix this issue by detecting and removing <slot> children when the dom-if hides.

sorvell pushed a commit that referenced this issue Dec 15, 2017
Fixes #4977. When templatized content is hidden (via `_showHideChildren`) as in `dom-if` top level `<slot>` elements are now removed instead of being hidden. This ensures that assigned nodes are not displayed in false-y dom-if elements.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants