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

[css-transforms-1] [css-transitions-1] Strange behavior with insertBefore vs appendChild and transitions #5334

Open
dead-claudia opened this issue Jul 19, 2020 · 10 comments
Labels
css-transforms-1 Current Work css-transitions-1 Current Work

Comments

@dead-claudia
Copy link

Originally filed in whatwg/html#5742, but it was recommended I post it here.

Relevant specs, though I'm not for certain which sections apply:

When using insertBefore with transitions and the FLIP technique, I get very strange results across browsers.

The only difference is the way the DOM nodes are rearranged, which is what confuses me about this.

Code for each

insertBefore:

<!doctype html>
<ul id="animate"><li>0</li><li>1</li><li>2</li><li>3</li></ul>
<script>
// Factored out so it's clearer what elements are added and removed

const animateRoot = document.getElementById("animate")
const elem0 = animateRoot.childNodes[0]
const elem1 = animateRoot.childNodes[1]
const elem2 = animateRoot.childNodes[2]
const elem3 = animateRoot.childNodes[3]

// ordered: [0, 1, 2, 3]
// mixed:   [2, 0, 3, 1]

function mix() { // ordered -> mixed
  setTimeout(order, 500)
  
  // First
  const first0 = elem0.getBoundingClientRect().top
  const first1 = elem1.getBoundingClientRect().top
  const first2 = elem2.getBoundingClientRect().top
  const first3 = elem3.getBoundingClientRect().top

  // Update [0, 1, 2, 3] -> [2, 0, 3, 1]
  animateRoot.insertBefore(elem2, elem0)
  animateRoot.appendChild(elem1)

  // Last + Invert
  // Note: earlier `transitionDuration` must be set before later
  // `transform`, or even more weirdness occurs (just try it and see
  // what happens).
  elem0.style.transitionDuration =
  elem1.style.transitionDuration =
  elem2.style.transitionDuration =
  elem3.style.transitionDuration = "0s"
  elem0.style.transform = `translateY(${first0 - elem0.getBoundingClientRect().top}px)`
  elem1.style.transform = `translateY(${first1 - elem1.getBoundingClientRect().top}px)`
  elem2.style.transform = `translateY(${first2 - elem2.getBoundingClientRect().top}px)`
  elem3.style.transform = `translateY(${first3 - elem3.getBoundingClientRect().top}px)`

  // Force re-layout
  document.body.offsetHeight

  // Play
  elem0.classList.add("transition")
  elem1.classList.add("transition")
  elem2.classList.add("transition")
  elem3.classList.add("transition")
  elem0.style.transform = elem0.style.transitionDuration =
  elem1.style.transform = elem1.style.transitionDuration =
  elem2.style.transform = elem2.style.transitionDuration =
  elem3.style.transform = elem3.style.transitionDuration = ""
}

function order() { // mixed -> ordered
  setTimeout(mix, 500)
  
  // First
  const first0 = elem0.getBoundingClientRect().top
  const first1 = elem1.getBoundingClientRect().top
  const first2 = elem2.getBoundingClientRect().top
  const first3 = elem3.getBoundingClientRect().top

  // Update [2, 0, 3, 1] -> [0, 1, 2, 3]
  animateRoot.insertBefore(elem1, elem3)
  animateRoot.insertBefore(elem2, elem3)

  // Last + Invert
  // Note: earlier `transitionDuration` must be set before later
  // `transform`, or even more weirdness occurs (just try it and see
  // what happens).
  elem0.style.transitionDuration =
  elem1.style.transitionDuration =
  elem2.style.transitionDuration =
  elem3.style.transitionDuration = "0s"
  elem0.style.transform = `translateY(${first0 - elem0.getBoundingClientRect().top}px)`
  elem1.style.transform = `translateY(${first1 - elem1.getBoundingClientRect().top}px)`
  elem2.style.transform = `translateY(${first2 - elem2.getBoundingClientRect().top}px)`
  elem3.style.transform = `translateY(${first3 - elem3.getBoundingClientRect().top}px)`

  // Force re-layout
  document.body.offsetHeight

  // Play
  elem0.classList.add("transition")
  elem1.classList.add("transition")
  elem2.classList.add("transition")
  elem3.classList.add("transition")
  elem0.style.transform = elem0.style.transitionDuration =
  elem1.style.transform = elem1.style.transitionDuration =
  elem2.style.transform = elem2.style.transitionDuration =
  elem3.style.transform = elem3.style.transitionDuration = ""
}

setTimeout(mix, 500)
</script>

appendChild:

<!doctype html>
<ul id="animate"><li>0</li><li>1</li><li>2</li><li>3</li></ul>
<script>
// Factored out so it's clearer what elements are added and removed

const animateRoot = document.getElementById("animate")
const elem0 = animateRoot.childNodes[0]
const elem1 = animateRoot.childNodes[1]
const elem2 = animateRoot.childNodes[2]
const elem3 = animateRoot.childNodes[3]

// ordered: [0, 1, 2, 3]
// mixed:   [2, 0, 3, 1]

function mix() { // ordered -> mixed
  setTimeout(order, 500)
  
  // First
  const first0 = elem0.getBoundingClientRect().top
  const first1 = elem1.getBoundingClientRect().top
  const first2 = elem2.getBoundingClientRect().top
  const first3 = elem3.getBoundingClientRect().top

  // Update [0, 1, 2, 3] -> [2, 0, 3, 1]
  animateRoot.appendChild(elem2)
  animateRoot.appendChild(elem0)
  animateRoot.appendChild(elem3)
  animateRoot.appendChild(elem1)

  // Last + Invert
  // Note: earlier `transitionDuration` must be set before later
  // `transform`, or even more weirdness occurs (just try it and see
  // what happens).
  elem0.style.transitionDuration =
  elem1.style.transitionDuration =
  elem2.style.transitionDuration =
  elem3.style.transitionDuration = "0s"
  elem0.style.transform = `translateY(${first0 - elem0.getBoundingClientRect().top}px)`
  elem1.style.transform = `translateY(${first1 - elem1.getBoundingClientRect().top}px)`
  elem2.style.transform = `translateY(${first2 - elem2.getBoundingClientRect().top}px)`
  elem3.style.transform = `translateY(${first3 - elem3.getBoundingClientRect().top}px)`

  // Force re-layout
  document.body.offsetHeight

  // Play
  elem0.classList.add("transition")
  elem1.classList.add("transition")
  elem2.classList.add("transition")
  elem3.classList.add("transition")
  elem0.style.transform = elem0.style.transitionDuration =
  elem1.style.transform = elem1.style.transitionDuration =
  elem2.style.transform = elem2.style.transitionDuration =
  elem3.style.transform = elem3.style.transitionDuration = ""
}

function order() { // mixed -> ordered
  setTimeout(mix, 500)
  
  // First
  const first0 = elem0.getBoundingClientRect().top
  const first1 = elem1.getBoundingClientRect().top
  const first2 = elem2.getBoundingClientRect().top
  const first3 = elem3.getBoundingClientRect().top

  // Update [2, 0, 3, 1] -> [0, 1, 2, 3]
  animateRoot.appendChild(elem0)
  animateRoot.appendChild(elem1)
  animateRoot.appendChild(elem2)
  animateRoot.appendChild(elem3)

  // Last + Invert
  // Note: earlier `transitionDuration` must be set before later
  // `transform`, or even more weirdness occurs (just try it and see
  // what happens).
  elem0.style.transitionDuration =
  elem1.style.transitionDuration =
  elem2.style.transitionDuration =
  elem3.style.transitionDuration = "0s"
  elem0.style.transform = `translateY(${first0 - elem0.getBoundingClientRect().top}px)`
  elem1.style.transform = `translateY(${first1 - elem1.getBoundingClientRect().top}px)`
  elem2.style.transform = `translateY(${first2 - elem2.getBoundingClientRect().top}px)`
  elem3.style.transform = `translateY(${first3 - elem3.getBoundingClientRect().top}px)`

  // Force re-layout
  document.body.offsetHeight

  // Play
  elem0.classList.add("transition")
  elem1.classList.add("transition")
  elem2.classList.add("transition")
  elem3.classList.add("transition")
  elem0.style.transform = elem0.style.transitionDuration =
  elem1.style.transform = elem1.style.transitionDuration =
  elem2.style.transform = elem2.style.transitionDuration =
  elem3.style.transform = elem3.style.transitionDuration = ""
}

setTimeout(mix, 500)
</script>

I've reproduced this behavior explained above on each of the following platforms:

  • Chrome 83.0.4103.116 on Windows 10 64-bit
  • Safari 13.1 on iOS 13
  • Firefox 78.0.2 on Windows 10 64-bit

Relevant framework bugs I've filed, before I narrowed it down further to this:


I suspect it's a bug in all three of those listed browsers, but I'm filing an issue here in case it's actually a spec issue or if a spec note is necessary for this.

@Loirooriol
Copy link
Contributor

See #3309. When you use appendChild or insertBefore, the node is temporarily taken out of the tree, and thus the transition is cancelled. Thus in you 1st code you are cancelling the transitions for all the elements, while in the 2nd one only for elem1 and elem2. elem0 and elem3 are choppy, I guess that's the reason, but I'm not an expert.

@annevk
Copy link
Member

annevk commented Jul 20, 2020

Thanks, that's probably it. Does CSS transitions detail this somehow?

@emilio
Copy link
Collaborator

emilio commented Jul 20, 2020

Yeah, that's it, and it is specified:

If an element is no longer in the document, implementations must cancel any running transitions on it and remove transitions on it from the completed transitions.

And also:

If an element is not in the document during that style change event or was not in the document during the previous style change event, then transitions are not started for that element in that style change event.

@annevk
Copy link
Member

annevk commented Jul 20, 2020

The first should maybe be reworded to "when an element is removed from the document" since the element might be in the document again when CSS next gets to look at it.

@dead-claudia
Copy link
Author

In case it was missed, the code snippets are literally just following the FLIP pattern minus the last step of removing the transition class after the transition completes. Here's some pseudocode for what each element is doing:

const initial = elem.getBoundingClientRect().top
moveElement(elem)
const final = elem.getBoundingClientRect().top
elem.style.setProperty("transition-duration", "0s")
elem.style.setProperty("transform", `translateY(${dx}px)`)
forceBrowserStyleRecalc()
elem.classList.add("transition") // contains `transition: 2s transform`
elem.style.removeProperty("transition-duration")
elem.style.removeProperty("transform")

The glitches seem to involve the starting point for the transform, as if dx isn't accurate somehow. And the only thing that's different between the two code snippets is how moveElement is performed.

@dead-claudia
Copy link
Author

dead-claudia commented Sep 11, 2020

Update: Found a simpler reproduction.

Movement primitives would not help fix this one, as the former does not add nodes with existing parents.

Edit: simplify it further.

@dead-claudia
Copy link
Author

And here's it simplified and deconstructed using two elements, one static and alternating remove/add, the other animated:

The only ones smooth are removing all nodes and re-adding the ones desired - no implicit removal is necessary. And it only affects nodes that are not removed. I've confirmed this in both Chrome and Firefox on Windows.

@dead-claudia
Copy link
Author

/cc @annevk in case you missed it

@annevk
Copy link
Member

annevk commented Sep 16, 2020

@isiahmeadows what would be interesting to know for this issue is if any of the cases you tested contradicts with the specification somehow.

@dead-claudia
Copy link
Author

dead-claudia commented Sep 16, 2020

That I'm not sure of, but I'm not nearly familiar enough with the relevant CSS specs to fully discern it.

Edit: My suspicion is that it's probably consistent with the spec, but my confidence level is like maybe at most 20%.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
css-transforms-1 Current Work css-transitions-1 Current Work
Projects
None yet
Development

No branches or pull requests

5 participants