Feature/duplicate tab #129
Conversation
| } | ||
| } | ||
|
|
||
| func duplicateTab(_ tab: Tab) { |
There was a problem hiding this comment.
I think this method could be simpler. it’s doing too much. There is already an open tab method that can be used to achive the same effect. Rearranging the tab position can also use the drag method that already is capable of inserting a tab at a specific positiion. Please try to reuse the existing methods.
|
Lint is failing too please address it |
…en new tab is created using duplicate
| self.lastAccessedAt = nowDate | ||
| } | ||
|
|
||
| func reorderTabs(from: Tab, to: Tab) { |
There was a problem hiding this comment.
Can you explain the need for modifiying this method?
There was a problem hiding this comment.
The old implementation attempts to achieve the new order solely by a series of property swaps, which is unnecessarily complicated and risky. The new approach directly manipulates the array structure to achieve the final order, making the logic much clearer and less prone to off-by-one errors.Highly faster as well The work is consistent: O(N) to re-assign the order values, regardless of how far the tab moves.
There was a problem hiding this comment.
Not really faster or cleaner. using insert and remove at index shifts the entire array twice plus reassigning the order - itrates over the entire list 3 times plus finding the index of from and to tab also goes over the list. so the method now is going over the list 5 times instead of just once in the prev case, which also stops if the swaping is complete using the break.
There was a problem hiding this comment.
Returned the Reorder function back to the original
|
|
||
| private func isInSameSection(from: Tab, to: Tab) -> Bool { | ||
| return from.type == to.type | ||
| } | ||
|
|
||
| private func moveTabBetweenSections(from: Tab, to: Tab) { | ||
| // Change the tab type to match the target section | ||
| from.type = to.type | ||
|
|
||
| // If moving to pinned or fav, save the URL | ||
| if to.type == .pinned || to.type == .fav { | ||
| from.savedURL = from.url | ||
| } else { | ||
| from.savedURL = nil | ||
| } | ||
| } | ||
|
|
||
| private func tabType(for section: TabSection) -> TabType { | ||
| switch section { | ||
| case .fav: | ||
| return .fav | ||
| case .pinned: | ||
| return .pinned | ||
| case .normal: | ||
| return .normal | ||
| } | ||
| } |
There was a problem hiding this comment.
TabUtils already has these methods, why the need to rewrite them here?
There was a problem hiding this comment.
Removed the duplicated function and used the one in Tab utils
ora/Services/TabDropDelegate.swift
Outdated
|
|
||
| private func isInSameSection(from: Tab, to: Tab) -> Bool { | ||
| return from.type == to.type | ||
| } | ||
|
|
||
| private func moveTabBetweenSections(from: Tab, to: Tab) { | ||
| // Change the tab type to match the target section | ||
| from.type = to.type | ||
|
|
||
| // If moving to pinned or fav, save the URL | ||
| if to.type == .pinned || to.type == .fav { | ||
| from.savedURL = from.url | ||
| } else { | ||
| from.savedURL = nil | ||
| } | ||
|
|
||
| // Reorder the tabs in the new section | ||
| from.container.reorderTabs(from: from, to: to) | ||
| } | ||
| } |
There was a problem hiding this comment.
Removed the duplicated function and used the one in Tab utils
There was a problem hiding this comment.
- The duplicated tab doesn’t load it needs to be clicked please call restoreTransientState method on it so it can load properly
- The order the duplicate tab is inserted is kind of weird please look into it. Ideally it should show up under the original tab
- The duplicate tab doesn’t work on dormat/inactive tabs so either we should make the option disabled or the function should actually work
…ntly' parameter to the tab opening method, allowing for silent initialization of web views based on user preferences.
…is dormant or not
* added duplication feature * fix : added calculation logic * fix : drag and reorder issue when duplicate tabs are located * fix : duplicate appears below duplicated tab instead of last * fix : fixed merge conflicts with main * fix : refactored duplicate method to use existing functions * fix : refactored open tab to return the tab and reordered the tabs when new tab is created using duplicate * fix : removed duplicate functions * fixed the reorder tab issue * Enhance TabManager to support silent loading of tabs. Added 'loadSilently' parameter to the tab opening method, allowing for silent initialization of web views based on user preferences. * feat : added is alive check when duplicating tab to check if the tab is dormant or not * feat : replaced is alive check with is web view ready --------- Co-authored-by: Kenenisa Alemayehu <[email protected]>
Changes in Detail
The signature of the openTab method has been modified to provide better utility to its callers:
Before: openTab(...) -> Void
After: openTab(...) -> Tab?
This allows the caller of openTab to immediately access and utilize the newly created Tab object (or nil if tab creation fails) without relying on external accessors or delegates.
The logic within the duplicateTab method has been simplified by leveraging the updated openTab method, resulting in cleaner and more concise code. This change reduces complexity and makes the duplication process easier to maintain.