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

[Bug] Top sites flash when removing one with frecent is disabled #15499

Closed
jonalmeida opened this issue Sep 28, 2020 · 1 comment
Closed

[Bug] Top sites flash when removing one with frecent is disabled #15499

jonalmeida opened this issue Sep 28, 2020 · 1 comment
Assignees
@codrut-topliceanu
Labels
🐞 bug Crashes, Something isn't working, .. eng:qa:verified QA Verified Feature:Shortcuts Top Sites/Topsites on the Firefox home page pin Issues, features, improvements that are still valid

Comments

@jonalmeida
Copy link
Contributor

jonalmeida commented Sep 28, 2020 •
edited by data-sync-user
Loading

Created from #14230 (comment)

Steps to reproduce

  1. Disable frecent top sites.
  2. Add a top site.
  3. Remove the top site.

Expected behaviour

  • When removing the top site, the animation should animate only the one top site removed.

Actual behaviour

  • All top sites animate instead.

Device information

  • Android device: Motorola Moto G6 (Android 8), Google Pixel 2 (Android 9), Nexus 5 (Android 6.0.1)
  • Fenix version: Nightly 200921 06:05 (Build #2015765171) GV 82.0a1 from 9/21

┆Issue is synchronized with this Jira Task
@jonalmeida jonalmeida added 🐞 bug Crashes, Something isn't working, .. Feature:Shortcuts Top Sites/Topsites on the Firefox home page labels Sep 28, 2020
@github-actions github-actions bot added the needs:triage Issue needs triage label Sep 28, 2020
@jonalmeida jonalmeida mentioned this issue Sep 28, 2020
Closed
@jonalmeida jonalmeida self-assigned this Sep 28, 2020
@gabrielluong gabrielluong removed the needs:triage Issue needs triage label Oct 11, 2020
@jonalmeida jonalmeida mentioned this issue Dec 14, 2020
Closed
3 tasks
@gabrielluong gabrielluong added the pin Issues, features, improvements that are still valid label Jan 8, 2021
@codrut-topliceanu codrut-topliceanu mentioned this issue Jun 14, 2021
Merged
3 tasks
@jonalmeida jonalmeida removed their assignment Jun 15, 2021
@codrut-topliceanu codrut-topliceanu self-assigned this Jun 15, 2021
codrut-topliceanu pushed a commit to codrut-topliceanu/fenix that referenced this issue Jun 28, 2021
For mozilla-mobile#15499 - Smoothly updates topSites list on remove
f443443 
 To remove the flash on refresh of the topsites list we have to use submitList, however using this too high up in the hierarchy of our listAdapters within listAdapters will cause children to refresh at once. The solution to this is to use submitList lower. Using it in TopSitesPagerAdapter.kt to update the TopSitesAdapter is the way to go. I've also had to use a dummy item for the "removed" Topsite ( with id = -1) so I can manually diff that before using submitList.
codrut-topliceanu pushed a commit to codrut-topliceanu/fenix that referenced this issue Jun 28, 2021
For mozilla-mobile#15499 - Smoothly updates topSites list on remove
4ad6cd7 
 To remove the flash on refresh of the topsites list we have to use submitList, however using this too high up in the hierarchy of our listAdapters within listAdapters will cause children to refresh at once. The solution to this is to use submitList lower. Using it in TopSitesPagerAdapter.kt to update the TopSitesAdapter is the way to go. I've also had to use a dummy item for the "removed" Topsite ( with id = -1) so I can manually diff that before using submitList.
@codrut-topliceanu codrut-topliceanu mentioned this issue Jun 28, 2021
Merged
3 tasks
codrut-topliceanu pushed a commit to codrut-topliceanu/fenix that referenced this issue Jul 8, 2021
For mozilla-mobile#15499 - Adds tests
eb2854f
codrut-topliceanu pushed a commit to codrut-topliceanu/fenix that referenced this issue Jul 8, 2021
For mozilla-mobile#15499 - Smoothly updates topSites list on remove
4320b6b 
 To remove the flash on refresh of the topsites list we have to use submitList, however using this too high up in the hierarchy of our listAdapters within listAdapters will cause children to refresh at once. The solution to this is to use submitList lower. Using it in TopSitesPagerAdapter.kt to update the TopSitesAdapter is the way to go. I've also had to use a dummy item for the "removed" Topsite ( with id = -1) so I can manually diff that before using submitList.
codrut-topliceanu pushed a commit to codrut-topliceanu/fenix that referenced this issue Jul 8, 2021
For mozilla-mobile#15499 - Adds tests
71c0f25
codrut-topliceanu pushed a commit to codrut-topliceanu/fenix that referenced this issue Jul 9, 2021
For mozilla-mobile#15499 - Removes unnecessary notifyItemRemoved
2db3f33
codrut-topliceanu pushed a commit to codrut-topliceanu/fenix that referenced this issue Jul 9, 2021
For mozilla-mobile#15499 - Removes unnecessary notifyItemRemoved
74ab9ee
codrut-topliceanu pushed a commit to codrut-topliceanu/fenix that referenced this issue Jul 20, 2021
For mozilla-mobile#15499 - Adds kdoc with to getChangePayload
d2cadae
codrut-topliceanu pushed a commit to codrut-topliceanu/fenix that referenced this issue Jul 20, 2021
For mozilla-mobile#15499 - Smoothly updates topSites list on remove
9d4c4c0 
 To remove the flash on refresh of the topsites list we have to use submitList, however using this too high up in the hierarchy of our listAdapters within listAdapters will cause children to refresh at once. The solution to this is to use submitList lower. Using it in TopSitesPagerAdapter.kt to update the TopSitesAdapter is the way to go. I've also had to use a dummy item for the "removed" Topsite ( with id = -1) so I can manually diff that before using submitList.
mergify bot pushed a commit that referenced this issue Jul 20, 2021
@mergify
For #15499 - Smoothly updates topSites list on remove
bc723e0 
 To remove the flash on refresh of the topsites list we have to use submitList, however using this too high up in the hierarchy of our listAdapters within listAdapters will cause children to refresh at once. The solution to this is to use submitList lower. Using it in TopSitesPagerAdapter.kt to update the TopSitesAdapter is the way to go. I've also had to use a dummy item for the "removed" Topsite ( with id = -1) so I can manually diff that before using submitList.
@codrut-topliceanu codrut-topliceanu mentioned this issue Jul 20, 2021
Closed
@lobontiumira
Copy link

Verified as fixed on the 92.0a1 Nightly build from 7/21 with Google Pixel (Android 10), and HTC 10 (Android 8).
I've encountered a crash after disabling the "Show most visited sites" and returning to the homescreen (filed #20449).

@lobontiumira lobontiumira added eng:qa:verified QA Verified and removed eng:qa:needed QA Needed labels Jul 21, 2021
czlucius pushed a commit to czlucius/fenix that referenced this issue Aug 20, 2021
@czlucius
For mozilla-mobile#15499 - Smoothly updates topSites list on remove
06a6589 
 To remove the flash on refresh of the topsites list we have to use submitList, however using this too high up in the hierarchy of our listAdapters within listAdapters will cause children to refresh at once. The solution to this is to use submitList lower. Using it in TopSitesPagerAdapter.kt to update the TopSitesAdapter is the way to go. I've also had to use a dummy item for the "removed" Topsite ( with id = -1) so I can manually diff that before using submitList.
czlucius pushed a commit to czlucius/fenix that referenced this issue Aug 22, 2021
@czlucius
For mozilla-mobile#15499 - Smoothly updates topSites list on remove
7293558 
 To remove the flash on refresh of the topsites list we have to use submitList, however using this too high up in the hierarchy of our listAdapters within listAdapters will cause children to refresh at once. The solution to this is to use submitList lower. Using it in TopSitesPagerAdapter.kt to update the TopSitesAdapter is the way to go. I've also had to use a dummy item for the "removed" Topsite ( with id = -1) so I can manually diff that before using submitList.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@codrut-topliceanu codrut-topliceanu

Labels
🐞 bug Crashes, Something isn't working, .. eng:qa:verified QA Verified Feature:Shortcuts Top Sites/Topsites on the Firefox home page pin Issues, features, improvements that are still valid
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@gabrielluong @jonalmeida @lobontiumira @codrut-topliceanu