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

Home screen search group removal tweaks #21871

Closed
grigoryk opened this issue Oct 12, 2021 · 1 comment
Closed

Home screen search group removal tweaks #21871

grigoryk opened this issue Oct 12, 2021 · 1 comment
Assignees

Comments

@grigoryk
Copy link
Contributor

grigoryk commented Oct 12, 2021

There are three problems with the group removals on home screen right now:

  1. it appears the UI update after removal races with the actual delete operation; it looks like the delete didn't happen (group is still there), but going in and out of home screen clears the group from the screen. the this was called out in the review but we didn't address it then.
  2. UI update after tapping "remove" scrolls the home screen to the top; it's quite jarring.
  3. after removing a search group, the same group still remains present in the tabs tray and in "jump back in" section.

For (1), we should dispatch a store update alongside async delete operations, to update the UI state right away.
For (2), try to see if we can avoid scrolling the screen.
For (3), we should "ungroup" existing tabs - we can dispatch a metadata key update action with searchTerms/referrer set to null. The behaviour this should achieve is the following: removing a search group from the home screen will ungroup related tabs in the tabs tray (and move them to "other tabs"), and remove the search from the "jump back in" section.

Patches for (1) and (3) will need to be uplifted to current beta 94.

┆Issue is synchronized with this Jira Task

@grigoryk grigoryk self-assigned this Oct 12, 2021
@github-actions github-actions bot added the needs:triage Issue needs triage label Oct 12, 2021
@grigoryk grigoryk removed the needs:triage Issue needs triage label Oct 12, 2021
grigoryk pushed a commit to grigoryk/fenix that referenced this issue Oct 13, 2021
…oup removal

Before this patch, this was the behavior - 'remove' button is clicked, we'd ask
the storage to remove metadata (on its IO thread), then navigate to Home
Screen.

This resulted in a race we could end-up on the Home Screen before delete
finishes, so the search groups do not appear to be removed (but,
refreshing the Home Screen again shows that they are removed).

This also resulted in an unnecessary navigation which felt very janky
(screen will "scroll" to the top) and was way more work than necessary.

After this patch, we:
 - dispatch two actions (on browserstore, on homefragmentstore) which
   remove the search groups from any relevant in-memory state; any UI bound to
   this state will be automatically "refreshed"
 - no longer navigate as part of the remove action, so the UI doesn't
   move and removal happens "in-place"
grigoryk pushed a commit to grigoryk/fenix that referenced this issue Oct 13, 2021
…oup removal

Before this patch, this was the behavior - 'remove' button is clicked, we'd ask
the storage to remove metadata (on its IO thread), then navigate to Home
Screen.

This resulted in a race we could end-up on the Home Screen before delete
finishes, so the search groups do not appear to be removed (but,
refreshing the Home Screen again shows that they are removed).

This also resulted in an unnecessary navigation which felt very janky
(screen will "scroll" to the top) and was way more work than necessary.

After this patch, we:
 - dispatch two actions (on browserstore, on homefragmentstore) which
   remove the search groups from any relevant in-memory state; any UI bound to
   this state will be automatically "refreshed"
 - no longer navigate as part of the remove action, so the UI doesn't
   move and removal happens "in-place"
grigoryk pushed a commit to grigoryk/fenix that referenced this issue Oct 13, 2021
…oup removal

Before this patch, this was the behavior - 'remove' button is clicked, we'd ask
the storage to remove metadata (on its IO thread), then navigate to Home
Screen.

This resulted in a race we could end-up on the Home Screen before delete
finishes, so the search groups do not appear to be removed (but,
refreshing the Home Screen again shows that they are removed).

This also resulted in an unnecessary navigation which felt very janky
(screen will "scroll" to the top) and was way more work than necessary.

After this patch, we:
 - dispatch two actions (on browserstore, on homefragmentstore) which
   remove the search groups from any relevant in-memory state; any UI bound to
   this state will be automatically "refreshed"
 - no longer navigate as part of the remove action, so the UI doesn't
   move and removal happens "in-place"
grigoryk pushed a commit to csadilek/fenix that referenced this issue Oct 13, 2021
…oup removal

Before this patch, this was the behavior - 'remove' button is clicked, we'd ask
the storage to remove metadata (on its IO thread), then navigate to Home
Screen.

This resulted in a race we could end-up on the Home Screen before delete
finishes, so the search groups do not appear to be removed (but,
refreshing the Home Screen again shows that they are removed).

This also resulted in an unnecessary navigation which felt very janky
(screen will "scroll" to the top) and was way more work than necessary.

After this patch, we:
 - dispatch two actions (on browserstore, on homefragmentstore) which
   remove the search groups from any relevant in-memory state; any UI bound to
   this state will be automatically "refreshed"
 - no longer navigate as part of the remove action, so the UI doesn't
   move and removal happens "in-place"
grigoryk pushed a commit to grigoryk/fenix that referenced this issue Oct 13, 2021
…oup removal

Before this patch, this was the behavior - 'remove' button is clicked, we'd ask
the storage to remove metadata (on its IO thread), then navigate to Home
Screen.

This resulted in a race we could end-up on the Home Screen before delete
finishes, so the search groups do not appear to be removed (but,
refreshing the Home Screen again shows that they are removed).

This also resulted in an unnecessary navigation which felt very janky
(screen will "scroll" to the top) and was way more work than necessary.

After this patch, we:
 - dispatch two actions (on browserstore, on homefragmentstore) which
   remove the search groups from any relevant in-memory state; any UI bound to
   this state will be automatically "refreshed"
 - no longer navigate as part of the remove action, so the UI doesn't
   move and removal happens "in-place"
grigoryk pushed a commit to grigoryk/fenix that referenced this issue Oct 13, 2021
…oup removal

Before this patch, this was the behavior - 'remove' button is clicked, we'd ask
the storage to remove metadata (on its IO thread), then navigate to Home
Screen.

This resulted in a race we could end-up on the Home Screen before delete
finishes, so the search groups do not appear to be removed (but,
refreshing the Home Screen again shows that they are removed).

This also resulted in an unnecessary navigation which felt very janky
(screen will "scroll" to the top) and was way more work than necessary.

After this patch, we:
 - dispatch two actions (on browserstore, on homefragmentstore) which
   remove the search groups from any relevant in-memory state; any UI bound to
   this state will be automatically "refreshed"
 - no longer navigate as part of the remove action, so the UI doesn't
   move and removal happens "in-place"
grigoryk pushed a commit that referenced this issue Oct 13, 2021
Before this patch, this was the behavior - 'remove' button is clicked, we'd ask
the storage to remove metadata (on its IO thread), then navigate to Home
Screen.

This resulted in a race we could end-up on the Home Screen before delete
finishes, so the search groups do not appear to be removed (but,
refreshing the Home Screen again shows that they are removed).

This also resulted in an unnecessary navigation which felt very janky
(screen will "scroll" to the top) and was way more work than necessary.

After this patch, we:
 - dispatch two actions (on browserstore, on homefragmentstore) which
   remove the search groups from any relevant in-memory state; any UI bound to
   this state will be automatically "refreshed"
 - no longer navigate as part of the remove action, so the UI doesn't
   move and removal happens "in-place"
@LaurentiuApahideanSV
Copy link

Verified as fixed on Firefox Preview 94.0.0-beta.4.

Devices used:

  • Huawei MediaPad M3 (Android 7)
  • Oppo Find X3 (Android 11)

pkirakosyan pushed a commit to gexsi/user-agent-android that referenced this issue Mar 7, 2022
…oup removal

Before this patch, this was the behavior - 'remove' button is clicked, we'd ask
the storage to remove metadata (on its IO thread), then navigate to Home
Screen.

This resulted in a race we could end-up on the Home Screen before delete
finishes, so the search groups do not appear to be removed (but,
refreshing the Home Screen again shows that they are removed).

This also resulted in an unnecessary navigation which felt very janky
(screen will "scroll" to the top) and was way more work than necessary.

After this patch, we:
 - dispatch two actions (on browserstore, on homefragmentstore) which
   remove the search groups from any relevant in-memory state; any UI bound to
   this state will be automatically "refreshed"
 - no longer navigate as part of the remove action, so the UI doesn't
   move and removal happens "in-place"
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants