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

For #28265 - Refactor InactiveTabs interactor/controller into TabsTray #28522

Merged
merged 2 commits into from
Jan 31, 2023
Merged

For #28265 - Refactor InactiveTabs interactor/controller into TabsTray #28522

merged 2 commits into from
Jan 31, 2023

Conversation

MozillaNoah
Copy link
Contributor

@MozillaNoah MozillaNoah commented Jan 12, 2023 •
edited by github-actions bot
Loading

PR 6 of 10 for #25722

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.

QA

  • QA Needed

To download an APK when reviewing a PR (after all CI tasks finished running):

  1. Click on Checks at the top of the PR page.
  2. Click on the firefoxci-taskcluster group on the left to expand all tasks.
  3. Click on the build-debug task.
  4. Click on View task in Taskcluster in the new DETAILS section.
  5. The APK links should be on the right side of the screen, named for each CPU architecture.

GitHub Automation

Fixes #28265

@MozillaNoah MozillaNoah requested review from a team as code owners January 12, 2023 22:49
@MozillaNoah
Copy link
Contributor Author

I'm going to move this to draft, as I found out from the UI test that doing this ticket created a circular dependency between BrowserTrayInteractor and TabsTrayInteractor, so I'll need to do #28266 first.

@MozillaNoah MozillaNoah marked this pull request as draft January 13, 2023 17:59
@mergify
Copy link
Contributor

mergify bot commented Jan 18, 2023

This pull request has conflicts when rebasing. Could you fix it @MozillaNoah? 🙏

@MozillaNoah MozillaNoah added the 🙅 waiting Issues that are blocked or has dependencies that are not ready label Jan 18, 2023
@MozillaNoah MozillaNoah force-pushed the move-inactive-tabs-interactor-to-tabs-tray branch from cf06914 to 9839d9d Compare January 25, 2023 00:09
@MozillaNoah MozillaNoah marked this pull request as ready for review January 25, 2023 00:10
@MozillaNoah MozillaNoah added needs:review PRs that need to be reviewed and removed 🙅 waiting Issues that are blocked or has dependencies that are not ready labels Jan 25, 2023
@MozillaNoah MozillaNoah force-pushed the move-inactive-tabs-interactor-to-tabs-tray branch from 9839d9d to 2dd4cbe Compare January 25, 2023 00:15
MatthewTighe
MatthewTighe approved these changes Jan 30, 2023
View reviewed changes
Copy link
Contributor

@MatthewTighe MatthewTighe left a comment

Choose a reason for hiding this comment

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

Had one question about scope, but no strong opinion on whether it should be resolved now or later. Otherwise, this LGTM!

app/src/main/java/org/mozilla/fenix/tabstray/browser/InactiveTabsInteractor.kt Show resolved Hide resolved
app/src/main/java/org/mozilla/fenix/tabstray/TrayPagerAdapter.kt Outdated Show resolved Hide resolved
@MozillaNoah MozillaNoah force-pushed the move-inactive-tabs-interactor-to-tabs-tray branch from 2dd4cbe to 5c8e436 Compare January 31, 2023 19:41
@MozillaNoah MozillaNoah added pr:approved PR that has been approved pr:needs-landing PRs that are ready to land [Will be merged by Mergify] and removed needs:review PRs that need to be reviewed labels Jan 31, 2023
@mergify mergify bot merged commit b8b7ad8 into mozilla-mobile:main Jan 31, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@t-p-white t-p-white t-p-white left review comments

@MatthewTighe MatthewTighe MatthewTighe approved these changes

@gabrielluong gabrielluong Awaiting requested review from gabrielluong

Assignees
No one assigned
Labels
pr:approved PR that has been approved pr:needs-landing PRs that are ready to land [Will be merged by Mergify]
Projects
None yet
Milestone
No milestone
4 participants
@MozillaNoah @gabrielluong @t-p-white @MatthewTighe