Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

[Bug] (regression) "tab closed" snackbar no longer disappears #7799

Closed
cadeyrn opened this issue Jan 18, 2020 · 17 comments
Closed

[Bug] (regression) "tab closed" snackbar no longer disappears #7799

cadeyrn opened this issue Jan 18, 2020 · 17 comments
Assignees
Labels
🐞 bug Crashes, Something isn't working, .. eng:qa:verified QA Verified Feature:Snackbar Feature:Tabs S2 Major Functionality/product severely impaired and a satisfactory workaround doesn't exist

Comments

@cadeyrn
Copy link
Contributor

cadeyrn commented Jan 18, 2020

Steps to reproduce

  1. close a tab
  2. open another open tab while the "tab closed" snackbar is still visible

Expected behavior

The snackbar disappears.

Actual behavior

The snackbar stays visible. Pressing the "undo" link closes the snackbar (but does not execute the "undo" action).

This is a recent regression.

Device information

  • Android device: OnePlus 7T Pro McLaren Edition / Android 10
  • Fenix version: master branch revision ffb2e72

┆Issue is synchronized with this Jira Task

@cadeyrn cadeyrn added the 🐞 bug Crashes, Something isn't working, .. label Jan 18, 2020
@github-actions github-actions bot added the needs:triage Issue needs triage label Jan 18, 2020
@cadeyrn cadeyrn changed the title [Bug] (regression) "tab closed" snackbar no longer disappers [Bug] (regression) "tab closed" snackbar no longer disappears Jan 18, 2020
@cadeyrn
Copy link
Contributor Author

cadeyrn commented Jan 18, 2020

@mcomella I bisected the issue. This is a regression from the second commit of #7423 (remove unnecessary CoordinatorLayout in fragment_home).

@djc
Copy link

djc commented Jan 21, 2020

I'm also seeing this.

@kbrosnan kbrosnan added Feature:Snackbar S2 Major Functionality/product severely impaired and a satisfactory workaround doesn't exist labels Jan 21, 2020
@ekager
Copy link
Contributor

ekager commented Jan 21, 2020

@mcomella could we make the root of activity_home.xml a coordinator layout instead of a linear layout? This should probably solve this problem without introducing more nested layouts?

@mcomella
Copy link
Contributor

@ekager Thanks for looping me in. I'm not sure that will work because the fragment container relies on the LinearLayout in order to dynamically size it when the Toolbar disappears and CoordinatorLayout is roughly a FrameLayout, according to the docs.

Is the CoordinatorLayout necessary to make snackbars go away? My understanding is that they should be independent over any particular view type. Perhaps it's getting attached to the wrong view?

@mcomella
Copy link
Contributor

I tried making the anchorView the root container of the activity, rather than bottom_bar, but now the Snackbar doesn't appear at all. I wonder if this would work correctly if we attached it to the root container of the home fragment.

Also, we may reintroduce the CoordinatorLayout in #7700 so it's possible this will be fixed with that patch.

@mcomella
Copy link
Contributor

In the old implementation, the Snackbar would be dismissed when switching between fragments. In the current implementation, the Snackbar remains. I'm not sure why this would be the case.

It's possible the previous findSuitableParent algorithm would choose the CoordinatorLayout at the top of the fragment view hierarchy but, now that there's no CoordinatorLayout, it's selecting the view at the top of the HomeActivity hierarchy, so it's not dismissed when changing fragments. I'm not sure why this would prevent the Snackbar from being hidden altogether.

@mcomella
Copy link
Contributor

mcomella commented Jan 22, 2020

I tried making the anchorView...

I noticed that anchorView was for which view to place the Snackbar above, not the view to which the Snackbar is bound. There is a separate view it's bound to (the first one passed in).

@mcomella
Copy link
Contributor

mcomella commented Jan 22, 2020

I confirmed the problem is fixed when I make the "suitable parent" of the SnackBar as R.id.homeLayout in FenixSnackbar.kt:

            do {
                if (view is ViewGroup && view.id == R.id.homeLayout) {
                    return view
                }

                if (view is CoordinatorLayout) {
                    // ...

I'm not sure what the "correct" solution is though.

@mcomella
Copy link
Contributor

Handing this off to @emalysz, who worked on the original implementation with me.

@emalysz
Copy link
Contributor

emalysz commented Jan 23, 2020

Should be fixed once #7878 lands

@andreicristianpetcu
Copy link

Quick workaround: swipe to delete removes the notification!

Screenshot_20200126-161308_Firefox_Nightly

@cadeyrn
Copy link
Contributor Author

cadeyrn commented Jan 26, 2020

Quick workaround: swipe to delete removes the notification!

Simply pressing the "undo" button also removes the snackbar (it does not execute the undo action). ;-)

@sv-ohorvath
Copy link
Contributor

Reproducing on Beta 3.2.0

This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🐞 bug Crashes, Something isn't working, .. eng:qa:verified QA Verified Feature:Snackbar Feature:Tabs S2 Major Functionality/product severely impaired and a satisfactory workaround doesn't exist
Projects
None yet
Development

No branches or pull requests