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

For 10503 - Update save to collection flow in tab tray #10523

Merged
merged 1 commit into from
May 12, 2020

Conversation

darkwing
Copy link
Contributor

@darkwing darkwing commented May 8, 2020

Fixes #10503

Pull Request checklist

  • Tests: This PR includes thorough tests or an explanation of why it does not
  • Screenshots: This PR includes screenshots or GIFs of the changes made or an explanation of why it does not
  • Accessibility: The code in this PR follows accessibility best practices or does not include any user facing features. In addition, it includes a screenshot of a successful accessibility scan to ensure no new defects are added to the product.

After merge

  • Milestone: Make sure issues finished by this pull request are added to the milestone of the version currently in development.

To download an APK when reviewing a PR:

  1. click on Show All Checks,
  2. click Details next to "Taskcluster (pull_request)" after it appears and then finishes with a green checkmark,
  3. click on the "Fenix - assemble" task, then click "Run Artifacts".
  4. the APK links should be on the left side of the screen, named for each CPU architecture

if (navController.currentDestination?.id == R.id.collectionCreationFragment) return

// Only register the observer right before moving to collection creation
// registerCollectionStorageObserver()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@boek I left this in the patch but commented out, as it's in the HomeFragment's implementation, but I don't understand what it's doing for us. Do we need this?

Also, upon saving to collection the tabs don't go away. Is that something we need to be doing?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't have collections on the tab tray so we don't need to register for that observer here!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And no, I think that behavior was confusing so we stopped removing the tabs that we were saving to the collection

@darkwing darkwing added the pr:work-in-progress PRs that are not ready to be reviewed yet and are actively being worked on label May 8, 2020
@darkwing darkwing requested a review from boek May 8, 2020 16:28
@darkwing darkwing self-assigned this May 8, 2020
@darkwing darkwing force-pushed the tab-tray-save-collection branch from 2167f10 to bc26bd3 Compare May 11, 2020 15:06
Copy link
Contributor

@boek boek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove that comment and 🚢

if (navController.currentDestination?.id == R.id.collectionCreationFragment) return

// Only register the observer right before moving to collection creation
// registerCollectionStorageObserver()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And no, I think that behavior was confusing so we stopped removing the tabs that we were saving to the collection

@darkwing darkwing force-pushed the tab-tray-save-collection branch from bc26bd3 to a4559bf Compare May 11, 2020 15:51
@darkwing darkwing force-pushed the tab-tray-save-collection branch from a4559bf to 7142bb5 Compare May 11, 2020 16:02
@darkwing darkwing removed the pr:work-in-progress PRs that are not ready to be reviewed yet and are actively being worked on label May 11, 2020
@darkwing darkwing merged commit 86dabe0 into mozilla-mobile:master May 12, 2020
@liuche liuche mentioned this pull request May 19, 2020
32 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tab Tray: Save a collection flow
2 participants