Skip to content

Conversation

Aniket-pd
Copy link
Contributor

@Aniket-pd Aniket-pd commented Sep 30, 2025

📜 Tickets

Jira ticket
Github issue

💡 Description

This PR refactors the “Close All Tabs” flow to fix the visual delay reported in #29046.

  • Added an isClosingAllTabs flag to ensure the flow only fires once.
  • On confirmation, launchCloseAllTabsAnimation() now starts the close-all animation immediately on the next main-queue turn, dismissing alerts first and deferring the actual purge until finishClosingAllTabs() runs after the animation completes.
  • Introduced performCloseAllTabsAnimation in TabDisplayView to provide a consistent scale/fade transition, skip the effect if Reduce Motion is enabled, and restore cell state if interrupted.
  • Updated removeAllTabs to batch tab removals:
    • Snapshot full tab list and selection before iteration (ensuring undo reflects the pre-close state).
    • Remove tabs in-memory (flushToDisk: false), then commit changes once.
    • Preserves analytics, counters, and toast hooks as a single event.

Result: Animations begin instantly when the user confirms, persistence and undo handling are more efficient, and main-thread blocking from repeated disk flushes is eliminated.

📝 Checklist

  • I filled in the ticket numbers and a description of my work
  • I updated the PR name to follow our PR naming guidelines
  • I ensured unit tests pass and wrote tests for new code
  • If working on UI, I checked and implemented accessibility (Dynamic Text and VoiceOver)
  • If adding telemetry, I read the data stewardship requirements and will request a data review
  • If adding or modifying strings, I read the guidelines and will request a string review from l10n
  • If needed, I updated documentation and added comments to complex code

@Aniket-pd Aniket-pd requested a review from a team as a code owner September 30, 2025 22:42
@Aniket-pd Aniket-pd requested a review from Foxbolts September 30, 2025 22:42
@Aniket-pd
Copy link
Contributor Author

i have added a custom closing animation that kicks in right after confirmation, so the transition feels instant while the actual tab removal happens afterwards

@Foxbolts Foxbolts requested a review from lmarceau October 1, 2025 20:51
@Foxbolts
Copy link
Collaborator

Foxbolts commented Oct 1, 2025

Hey @Aniket-pd, thank you for another contribution! Tagging @lmarceau as she has more familiarity with the tab tray.

@lmarceau
Copy link
Contributor

lmarceau commented Oct 2, 2025

I'll have a first look shortly to provide some feedback, but FYI we'll need feedback as well from @mattreaganmozilla since he had an extensive look into that particular problem already

@lmarceau lmarceau changed the title [iOS 26] Fix delay in “Close All Tabs” action (#29046) Refactor FXIOS-13352 [Tab Manager] Fix delay in “Close All Tabs” action (#29046) Oct 2, 2025
Copy link
Contributor

@lmarceau lmarceau left a comment

Choose a reason for hiding this comment

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

Hello! Had a look and I understand part of this PR is an animation to improve the "Close all tabs", but the original issue is really an optimization problem of the closing tabs functionality. I just added further information on the ticket itself on how to replicate.

Here is a video showcasing the problem. After I click the "Close all tabs", the application freezes. This is on the current branch, so the problem is still present with the suggested changes. The animation IMO is not helping this particular problem.

Simulator.Screen.Recording.-.iPhone.17.Pro.-.2025-10-02.at.20.00.59.mov

@mattreaganmozilla
Copy link
Collaborator

Thanks @lmarceau for the context/comments. Linking this here just to cross-reference: #26782

@Aniket-pd Aniket-pd force-pushed the bugfix/close-all-tabs-delay branch from 8fcc50c to 3bd6d48 Compare October 10, 2025 08:42
@Aniket-pd Aniket-pd closed this Oct 10, 2025
@Aniket-pd Aniket-pd force-pushed the bugfix/close-all-tabs-delay branch from 3bd6d48 to 9ed40e5 Compare October 10, 2025 08:57
@Aniket-pd
Copy link
Contributor Author

hey @lmarceau! , I’ve opened a new PR with a different approach to this fix , continuing from here: #29974

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants