-
Notifications
You must be signed in to change notification settings - Fork 448
Implement Bookmark All Tabs Long-Press Action #4133
Conversation
Client/Frontend/Browser/BrowserViewController/BrowserViewController+ToolbarDelegate.swift
Outdated
Show resolved
Hide resolved
...ntend/Browser/Toolbars/BottomToolbar/Menu/Bookmarks/AddEditBookmarkTableViewController.swift
Outdated
Show resolved
Hide resolved
...ntend/Browser/Toolbars/BottomToolbar/Menu/Bookmarks/AddEditBookmarkTableViewController.swift
Outdated
Show resolved
Hide resolved
style: .default, handler: { [unowned self] _ in | ||
|
||
let addBookMarkController = AddEditBookmarkTableViewController( | ||
mode: BookmarkEditMode.addFolderUsingTabs(title: Strings.savedTabsFolderTitle, tabList: tabManager.allTabs)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if you have 2 tabs opened in regular mode, 3 tabs in private mode and you add them?
Please double check this code works well in this case, priv mode tabs and regular tabs should be added and counted separately
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perhaps tabsForCurrentMode
should be the correct getter, not tabManager.allTabs
, haven't checked it though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tabsForCurrentMode
is the correct getter just checked.
Fix at cbe4bfc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Such a nice feature in less than 100 lines, good job :)
Summary of Changes
This pull request fixes #4132
Submitter Checklist:
NSLocalizableString()
Test Plan:
Screenshots:
Reviewer Checklist:
QA/(Yes|No)
bug
/enhancement