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

Visibility not restored for Hero after pop #40239

Closed
hahafather007 opened this issue Sep 11, 2019 · 32 comments · Fixed by #40306
Closed

Visibility not restored for Hero after pop #40239

hahafather007 opened this issue Sep 11, 2019 · 32 comments · Fixed by #40306
Assignees
Labels
a: animation Animation APIs c: regression It was better in the past than it is now f: routes Navigator, Router, and related APIs. framework flutter/packages/flutter repository. See also f: labels.

Comments

@hahafather007
Copy link

After clicking the Hero widget to jump to a list page, scroll down the list until the Hero widget of the corresponding tag leaves the screen and then return to the page. The Hero widget of the previous page will be blank. This is a very serious problem, and now our online project has been plagued by this problem.
QQ20190911-173956@2x
QQ20190911-174016@2x
Flutter version:

[✓] Flutter (Channel beta, v1.7.8+hotfix.4-pre.7, on Mac OS X 10.14.6 18G87,
    locale zh-Hans-CN)
[✓] Android toolchain - develop for Android devices (Android SDK version 28.0.3)
[✓] Xcode - develop for iOS and macOS (Xcode 10.3)
[✓] Android Studio (version 3.5)
[!] IntelliJ IDEA Community Edition (version 2019.2.1)
    ✗ Flutter plugin not installed; this adds Flutter specific functionality.
[✓] Connected device (1 available)

Reproduce code like this:

import 'package:flutter/material.dart';

void main() => runApp(MyApp());

class MyApp extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      home: MyHomePage(),
    );
  }
}

class MyHomePage extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(),
      body: Center(
        child: GestureDetector(
          onTap: () {
            Navigator.of(context).push(MaterialPageRoute(builder: (context) {
              return Scaffold(
                body: ListView(
                  children: <Widget>[
                    Center(
                      child: Hero(
                        tag: "111",
                        child: Container(
                          width: 50,
                          height: 50,
                          color: Colors.blue,
                        ),
                      ),
                    ),
                    Container(
                      height: 2000,
                    ),
                  ],
                ),
              );
            }));
          },
          child: Hero(
            tag: "111",
            child: Container(
              width: 50,
              height: 50,
              color: Colors.blue,
            ),
          ),
        ),
      ),
    );
  }
}
@hahafather007
Copy link
Author

@BondarenkoStas BondarenkoStas added framework flutter/packages/flutter repository. See also f: labels. severe: customer blocker labels Sep 11, 2019
@LongCatIsLooong LongCatIsLooong added the f: routes Navigator, Router, and related APIs. label Sep 11, 2019
@LongCatIsLooong LongCatIsLooong self-assigned this Sep 11, 2019
@goderbauer
Copy link
Member

@chunhtai This may be caused by your change in #37341. Maybe you can take a look?

@goderbauer goderbauer changed the title There is a serious bug with Hero widget!!! Visibility not restored for Hero after pop Sep 11, 2019
@LongCatIsLooong
Copy link
Contributor

LongCatIsLooong commented Sep 11, 2019

@goderbauer It's probably not introduced in that PR. We didn't handle the case where fromHero is null so the hero transition technically lasts forever. Working on a fix now.

@goderbauer
Copy link
Member

But prior to that PR we would restore the visibility of the from Hero once the flight is completed so it would always be visible when you do the pop, no?

@LongCatIsLooong
Copy link
Contributor

Sorry I phrased it poorly. We set offstage to false when the animation finishes (i.e. in an animation status change callback) but there's no animation to begin with because the fromHero is null (garbage collected by the ListView).

@chunhtai
Copy link
Contributor

I wonder what is the expected behavior of this situation
There are two possible scenarios.
1, the fromHero is destroyed
2, the fromHero is keptalive but outside viewport.

@LongCatIsLooong
Copy link
Contributor

My original plan was to fade in the toHero (since we're fading out the hero if the flight is aborted) but it doesn't look as good as I expected because the hero OverlayEntry is the topmost OverlayEntry, so it's entirely visible during the transition, even when the from route overlaps with it. So I'm planning to end the hero transition right away if fromHero is null. Thoughts?

@chunhtai
Copy link
Contributor

@LongCatIsLooong I think ending it if fromHero is null make sense. What about the fromHero is keptalive, will the hero transition trigger in this case?

@nekocode
Copy link

We met the same problem. Hope you guys can fix it soon. Otherwise we can only downgrade the flutter sdk.

@LongCatIsLooong
Copy link
Contributor

LongCatIsLooong commented Sep 13, 2019

@chunhtai it produces an interesting animation, but that seems to be a benigner bug. Created #40307.

@LongCatIsLooong
Copy link
Contributor

LongCatIsLooong commented Sep 13, 2019

Oh I didn't realize #37341 keeps the original Hero invisible.

@dshukertjr
Copy link

Why is this issue closed? It is not solved.

@LongCatIsLooong
Copy link
Contributor

@dshukertjr are you using the master channel? If you are and the issue is still reproducible, could you provide a minimal repro?

@dshukertjr
Copy link

@LongCatIsLooong No, I'm using the stable channel. Is the issue solved in the master channel?

@LongCatIsLooong
Copy link
Contributor

@dshukertjr it should be. The issue was closed because #40306 was merged.

@aleksandar-radivojevic
Copy link

This issue is not solved in 1.9.1+hotfix.4 !!!!

@vitor-gyant
Copy link

@LongCatIsLooong I'm still facing this issue on stable branch version 1.9.1+hotfix4. Could you please double check if the issue is solved in this version ? Thanks.

@LongCatIsLooong
Copy link
Contributor

LongCatIsLooong commented Oct 9, 2019

@vitor-gyant it is not in 1.9.1. The earliest version with the fix is v1.10.4: 6430012.

@vitor-gyant
Copy link

@LongCatIsLooong I see. Any plans to cherry pick it to 1.9 on the stable branch ?

@LongCatIsLooong
Copy link
Contributor

LongCatIsLooong commented Oct 9, 2019

Unfortunately, not that I know of. You could try applying the patch locally, or consider switching to beta/dev/master: https://github.com/flutter/flutter/wiki/Flutter-build-release-channels.

@iassal
Copy link

iassal commented Oct 10, 2019

This caused a severe regression in our app as we use hero animations for input fields. Had to deprecate all pop calls as a hotfix

@LongCatIsLooong
Copy link
Contributor

LongCatIsLooong commented Oct 18, 2019

/cc @tvolkert for the hotfix request. See also the duplicates of this issue.

@LongCatIsLooong
Copy link
Contributor

FYI the hotfix request is being evaluated.

@vitor-gyant
Copy link

@tvolkert Hot fix is a must for us, at least. Keeping a fork of the flutter repo just to be able to apply the cherry pick creates a big maintenance issue. Moreover, our app uses several hero animations in the primary flows and now they simply don't work and the visual impact is a blocker for the release.

We've started the migration process to the master branch but it will take time to finish plus the QA rounds. Since we also have app2app changes on master this will slow down us for a while if the hot fix is not approved.

goderbauer pushed a commit to goderbauer/flutter that referenced this issue Oct 22, 2019
This fixes the issue described in flutter#40239:

* Restore offstage and ticker mode after hero pop and the from hero is null (flutter#40306)
goderbauer added a commit that referenced this issue Oct 23, 2019
This fixes the issue described in #40239:

* Restore offstage and ticker mode after hero pop and the from hero is null (#40306)
@chickenblood
Copy link
Contributor

What is the status of this? Is there a workaround? This bug is affecting my new app which I hoped to get out on December 1. It seems like a common pattern that would affect many apps.

@chickenblood
Copy link
Contributor

Nevermind. I just did flutter upgrade and it looks like it has made it into stable.

@limyeanfen
Copy link

limyeanfen commented Nov 27, 2019

@chickenblood

Nevermind. I just did flutter upgrade and it looks like it has made it into stable.

May i know which version you upgraded ?

@iassal
Copy link

iassal commented Dec 3, 2019

@limyeanfen I think it was fixed in Flutter 1.9.1+hotfix.6 as per the head commit here https://github.com/flutter/flutter/releases/tag/v1.9.1%2Bhotfix.6

@Deepanshu-Rohilla
Copy link

It is not fixed in the latest version

@violet-dev
Copy link

How to fix it?

@LongCatIsLooong
Copy link
Contributor

The bug you're seeing is likely a different bug, since this one has long been hotfixed on the stable branch (unless you're still on a version older than 1.9.1). Could you file a new issue using this link?

@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a: animation Animation APIs c: regression It was better in the past than it is now f: routes Navigator, Router, and related APIs. framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Development

Successfully merging a pull request may close this issue.