Open avimar opened 4 years ago
Weirdly this is only with slide. Fade works as expected.
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:
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
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.
@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?
@BillyLevin I see the issue, but it looks pretty good to me.
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.
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.
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."
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.
I wrote up a quick proof-of-concept PR (https://github.com/sveltejs/svelte/pull/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.
I'm also running into this.
Ran into this just now. PR is ready cmon guys ðŸ¤
Just wanted to note that since my PR above, there's been some improvements introduced to CSS (and browser support) to tackle this issue: https://developer.chrome.com/blog/entry-exit-animations
Perhaps that approach is cleaner. I haven't tried it myself, but I'd imagine it would be.
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.