-
Notifications
You must be signed in to change notification settings - Fork 1.3k
For #11498 and #11499 - Add Sync tabs error view (with/without CTA) #11634
For #11498 and #11499 - Add Sync tabs error view (with/without CTA) #11634
Conversation
Codecov Report
@@ Coverage Diff @@
## master #11634 +/- ##
============================================
+ Coverage 27.51% 27.57% +0.06%
- Complexity 1006 1010 +4
============================================
Files 415 416 +1
Lines 16586 16655 +69
Branches 2127 2134 +7
============================================
+ Hits 4563 4592 +29
- Misses 11697 11732 +35
- Partials 326 331 +5
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Thanks.
I found some bugs unrelated to your PR that I will document in their own separate issues.
- Cold starting Fenix and rushing to synced tabs fragment leads to some strange concurrency issues.
- Being logged in on other devices with no tabs opened on them will lead to a blank page of synced devices instead of the "no tabs error".
Both issues are visible in this video : https://drive.google.com/file/d/14u83g7dp8Kjkw2G7LHjYdYJgbEGQ69Na/view?usp=sharing
Currently waiting on mozilla-mobile/android-components#7422 to get fixed on AC |
We have two bugs filed that needs more account manager support: |
@BranescuMihai Related AC issue/PR is now closed/merged |
8128aff
to
b257b6b
Compare
Progress: Still not merging this because there are some issues on AC that call |
Product wants this CTA in our Sept release. Can we prioritize the blocking issues in AC or find a workaround @jonalmeida @BranescuMihai ? |
Since the related bug is now explained in a different issue: #12195, and this PR is only about a UI change, we can proceed with this one |
b257b6b
to
db8fd83
Compare
As per this documentation:
Sign-in error
No tabs error
Re-authenticate error
Pull Request checklist
After merge
To download an APK when reviewing a PR: