Closed Snonky closed 11 months ago
There is error 'package:flutter/src/rendering/object.dart': Failed assertion: line 2597 pos 12: '!_debugDisposed': is not true. same error but at the same time I have another thing when I use LocalHero without Key it doesn't work is that problem or bug or it is necesseity 'package:flutter/src/rendering/layer.dart': Failed assertion: line 2406 pos 16: '_debugPreviousLeaders!.isEmpty': is not true. this is the error that is showing
The first error seems related to issue #5. I also have a PR for that #16.
In my forked branch feature/only-remount
I have merged both this and the other PR. See https://github.com/letsar/local_hero/issues/1#issuecomment-1742946384.
when I use LocalHero without Key it doesn't work is that problem or bug or it is necesseity
Yes, you have to use a key on each local hero. Without the key, the change in position cannot be tracked.
Thanks for this PR! I tried it by surrounding _DraggableExample
with
return SingleChildScrollView(
child: SizedBox(
height: 2000,
I saw at least two issues, one is performance : the scroll does not seem smooth. The other one is as we scroll, the widgets are painted above the scaffold and this if we scroll too far, the are painted above the appBar. With this PR you are maybe in the right direction though, thank you!
The first issue I did not notice because I only used a Windows PC with a GPU where I get 144 FPS. What device did you use?
For the second issue you can try wrapping the SingleChildScrollView
with your LocalHeroOverlay
. That should clip the overflowing widgets.
My bad, I woke up today thinking about your PR and I remembered that I completely forgot to set onlyAnimateRemount
to true
😓 . Sorry for that. It works really great after this 🚀 .
I need to look at the code but this is a really great idea, thanks!
Do you see any reason to not make onlyAnimateRemount
true by default?
I completely forgot to set onlyAnimateRemount to true 😓 . Sorry for that.
Don't worry about it, that also happened to me multiple times during testing! 😅
Do you see any reason to not make onlyAnimateRemount true by default?
Mainly backwards compatibility. But you are right this is unlikely to break any intended functionality. If there will be a short migration guide I agree with you, it can be true
by default.
The second smaller reason is ListView
. It doesn't work out of the box with LocalHero
because the items that are scrolled away are first cached and then disposed. This is good for performance but it breaks the local hero tracking. The cached items (those that are just a few pixels out of the scroll view) are always layed out but not painted. But since the LocalHeroController
gets updates about the widget position via the paint call it goes out of sync with the layout position.
I already have a solution for this that I will post as a follow-up PR when we reviewed this one: A CustomScrollView
with a SliverLocalHeroList
that behaves just like a normal SliverList
(this is what ListView
uses) but it paints its LocalHero
children even when they are in the cache region. Then it works with minimal performance penalty.
Yeah we cam bump the version to 0.3.0 to indicate there is a possible breaking change. Can you resolve the conflict so that we can merge this PR?
For the issue with slivers, can you post an example where it does not work? I made a simple test with a ListView
but I don't see any issue for the moment if I wrap the item with a StatefulWidget
having its State
applying the AutomaticKeepAliveClientMixin
.
Can you resolve the conflict ✔
I added a new tab to the example app. To see the issue do the following:
You will see a one frame glitch where the items are in the wrong position.
Another problem comes up when pressing 'Add Item'. The last visible item does not get animated off the screen, it just disappears instantly.
If your idea with the keepalive fixes this let me know. I'm not sure how to test it myself-
Oh ok I see the issue thanks, and no AutomaticKeepAliveClientMixin
won't help with that.
Thanks for this PR by the way, this is a great feature!
Issue #1 has a few requests for a LocalHero that works under a scrollable widget.
I think I found a working solution that should give LocalHeros the ability to skip their animation when scrolling.
It adds the
onlyAnimateRemount
parameter to theLocalHeroScope
constructor. Set it totrue
and the LocalHeros in this scope wont animate unless their widget in the widget tree and their position on screen changed.This will at least work with
SingleChildScrollView
scrollables because these don't do tree surgery.Before & after videos
Before: ![local_hero_before](https://github.com/letsar/local_hero/assets/47639380/a5d75f66-bef7-4e17-b53c-ec2102a9fc7c) After: ![local_hero_after](https://github.com/letsar/local_hero/assets/47639380/d12b46c7-87d1-47da-8f02-e196abf8e528)