firefox-ios
firefox-ios copied to clipboard
Bugfix FXIOS-9473 [Tab Tray Refactor] Scroll to restored close-undo tab
:scroll: Tickets
:bulb: Description
This PR adds new behaviour to the tab tray. When a tab is closed, a toast appears. If the user taps "Undo" on the toast, the tab is restored. However, sometimes the tab is restored out of the view, either because the user has scrolled, or the tab was restored in a new row at the bottom of the tab tray (off screen).
Now when the user taps "Undo", the restored tab will be scrolled onto the screen if it would not be 100% visible within the collectionView's frame.
Additional bugfixes/refactoring were done to fix some redux state issues (which triggered newState of the TabDisplayView to fire too often) and to make it less likely to pass bad indices to the scroll action of the TabDisplayView.
Breakdown of Changes
- Added a more discrete scroll
ScrollStateto the TabsPanelState - Created a new
TabPanelMiddlewareActionType.scrollToTabtype for more fine-grained scroll control - Reworked scroll behavior so that the TabsDisplayView scrolls only state transformations from .scrollToTab and .willAppearTabPanel actions
- Added
createTabScrollBehavior, which adds logic to decide how to scroll and which tab index/section to scroll to, given the TabsStatePanel state - Added
createTabScrollBehaviorunit tests - Fixed a
TabsPanelStateredux bug which caused unnecessary duplicate calls to TabDisplayView's newState() method (don't return the previous state, always return a new state with transient values like animation triggers cleared) - Fixed another bug in which The TabDisplayView was triggering state updates... even when it wasn't the current visible panel (e.g. private tabs panel shouldn't update for the normal tabs panel state changes)
Ultimately, these changes should also help with the DiffableDataSource work as we are being more careful about when the TabDisplayView updates with TabsPanelState changes, and we're being more careful with the indices we pass. As well, there are extra checks in the code to avoid collectionView.scrollToItem(at:at:animated:) crashes.
Demo
https://github.com/user-attachments/assets/6af196f3-9692-4586-b673-0ccc107db6b0
:pencil: Checklist
You have to check all boxes before merging
- [x] Filled in the above information (tickets numbers and description of your work)
- [x] Updated the PR name to follow our PR naming guidelines
- [x] Wrote unit tests and/or ensured the tests suite is passing
- [ ] When working on UI, I checked and implemented accessibility (minimum Dynamic Text and VoiceOver)
- [ ] If needed, I updated documentation / comments for complex code and public methods
- [ ] If needed, added a backport comment (example
@Mergifyio backport release/v120)
This is nearly ready to for a review. I want to write a few TabPanelState unit tests, and I also need to just double check the TabDisplayPanel doesn't do anything funny in newState() for synced tabs.
I have another PR coming which fixes the toast behaviour as that needed to be in a higher state than the individual tab panels to avoid some janky stuff happening in the individual TabPanelViews (plus I found another of those redux state bugs in the TabTrayState as well which was causing another bug).
| Messages | |
|---|---|
| :book: | Project coverage: 32.66% |
| :book: | Edited 12 files |
| :book: | Created 1 files |
Client.app: Coverage: 30.49
| File | Coverage | |
|---|---|---|
| TabDisplayView.swift | 28.74% | ⚠️ |
| TabManagerMiddleware.swift | 5.75% | ⚠️ |
| TabsPanelState.swift | 62.43% | ✅ |
| TabPanelAction.swift | 100.0% | ✅ |
| InactiveTabsModel.swift | 58.33% | ✅ |
| TabModel.swift | 100.0% | ✅ |
Generated by :no_entry_sign: Danger Swift against e2df157bd318cc005d294a672db5e594e2a70705