Transitions don't work on table row addition

[avimar]

Describe the bug
When I add a row to a table, even when the table has transition: set, it just pops in ignoring the transition. Oddly, it works on removing the row.

Logs
No logs.

To Reproduce
Reproducible bug: https://svelte.dev/repl/f7fefbd5eb63471db69a89bf36398d38?version=3.23.0

Based on demo: https://svelte.dev/tutorial/local-transitions which uses divs

Expected behavior
Transitions should work on both adding a removing a table row.

Information about your Svelte project:

• Your browser and the version: Google Chrome Version 83.0.4103.61 (Official Build) (64-bit)

• Svelte version 3.23

repl, none

Severity
Not much, it's just some coolness in transitions with adding. But it does help the user acclimate to the changes on the screen.

[antony]

Weirdly this is only with slide. Fade works as expected.

[BillyLevin]

The problem with this example specifically is that changing the height of a tr element doesn't do anything. You can set it to 0 and it'll still look the same. What does work is animating the line-height, but this has some problems:

• The default value is normal, and it's pretty much up to the browser what this value is, so we have to guess. It's apparently around 1.2em (MDN reference), so can estimate by just multiplying the font size by 1.2, but wouldn't necessarily be 100% accurate

• Animating the line height looks weird because the text stays the same size until it's gone:

• If we animate the font size too, it solves the above problem, but it also looks weird because the text shrinks towards the top left rather than in the same direction as the transition:

So yeah, unless someone else has a better solution, this seems pretty hacky and I would guess unwanted? Although, it could be justified if only being applied to this edge case with table elements. Let me know if worth sending in a PR

[avimar]

Wow, ok that's pretty complicated. Great work on tracking that down @BillyLevin and very nice screenshots to explain it!

My guess is that since the work-around has it's own issues, we should just update the docs to warn that slide doesn't work on adding <tr> so choose another option.

[BillyLevin]

@avimar I managed to come up with a slightly better version, but you can see on the reverse transition, there's a bit of a gap at the bottom being caused by the child td before it gets removed.

@antony do you think this is worth adding for tr elements only? or just a docs/compiler warning saying to use a different transition?

[avimar]

@BillyLevin I see the issue, but it looks pretty good to me.

[antony]

I'm by no means an expert on transitions, so this is a good catch.

I'm wondering if there is something we can do to mask the fact that slide won't work on that element, maybe even as a developer warning, rather than adding loads of caveats to the documentation.

[Conduitry]

Not in a reasonable way that I can think of that wouldn't add extra code to everyone's apps. We don't want the compiler to make any assumptions about the contents of any of the stock svelte/... imports, nor are we able to look into user-defined transitions - and there's not a mechanism to have dev-mode-only code in these .js imports - without requiring everyone to add a particular rollup-plugin-replace/etc plugin to their bundler configuration, which would be a breaking change.

[mattwolff]

This won't be limited to Table Rows, pretty confident this will be the case on any element that doesn't have a Block Level display property... For instance, span and img elements are set to display: inline by default.

You can see the same behavior described above with the tr element happening with other inline-ish elements in this REPL. If (on line 43) you change the span element's display property to block inline-block or even flex, grid, or any other block-ish layout, the transition works as expected.

A possible developer warning (I imagine) would only need to be something along the lines of: "The slide transition animates the height of an element. That being said, it will only work properly with block-level elements: those that can accept a height within document flow."

[mattwolff]

Specifically with sliding a <tr> into view... I'd suggest the same: set it to display: block or even wrap it in a <div> and apply the slide transition to that wrapping element. Like this REPL.

[chrisirhc]

I wrote up a quick proof-of-concept PR (#5922) to apply CSS throughout animation (display: block, in this case).

Would appreciate thoughts on this approach. I get that it does add some complexity in the animation/transitions. However, I can also foresee it might be helpful for custom transitions that require custom non-animatable CSS.

I'd like a quick sanity check on the PR.