NSPopover-based Bookmarks menu system #3106
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
There was a problem hiding this comment.
@mallexxx This is a first pass; overall, the functionality looks good. I still need to go through all the code changes, given that there were a lot of additions.
My biggest questions code related is: why do we not create a new view controller instead of reusing the BookmarkListViewController? BookmarkListViewController is huge now and hard to follow.
| @@ -1,7 +1,7 @@ | |||
| <?xml version="1.0" encoding="UTF-8"?> | |||
| <Scheme | |||
| LastUpgradeVersion = "1530" | |||
| version = "1.8"> | |||
| version = "1.7"> | |||
There was a problem hiding this comment.
@mallexxx This seems like an unintended change. We should revert this
There was a problem hiding this comment.
this was a change made by Xcode 15.2; 15.4+ always reverts it back to "1.7"
jotaemepereira
approved these changes
Aug 22, 2024
1 task
Group&align code, add MARK-s, fix naming, cleanup
…#3110) - Add "Open all in new tabs" menu items - Fix bookmarks menu not closed when nested bookmarks menu item (or open all action) selected
- Bookmarks drag&drop support unified to act the same way in different places of UI: drag&drop support moved to BookmarkDragDropManager - Implemented drag&drop support for the Bookmarks Bar menus
- Add keyboard controls support to Bookmarks Bar menu - Activate search by typing any letter in the Navigation Bar Bookmarks menu - Close search in the Navigation Bar Bookmarks menu first on Esc press - Switching between the search bar and the search results using keyboard in the Navigation Bar Bookmarks menu
Adjust Bookmarks Menu popover size when number of items changes
Task/Issue URL: https://app.asana.com/0/1206488453854252/1207733191644528/f Tech Design URL: https://app.asana.com/0/481882893211075/1207774401221083/f CC: **Description**: Adds new pixels to indicate how many opt outs are successfully submitted within 24 hours **Steps to test this PR**: 1. With opt out jobs already existing (older than 24 hours), check the pixels fire appropriately <!-- Tagging instructions If this PR isn't ready to be merged for whatever reason it should be marked with the `DO NOT MERGE` label (particularly if it's a draft) If it's pending Product Review/PFR, please add the `Pending Product Review` label. If at any point it isn't actively being worked on/ready for review/otherwise moving forward (besides the above PR/PFR exception) strongly consider closing it (or not opening it in the first place). If you decide not to close it, make sure it's labelled to make it clear the PRs state and comment with more information. --> **Definition of Done**: * [x] Does this PR satisfy our [Definition of Done](https://app.asana.com/0/1202500774821704/1207634633537039/f)? --- ###### Internal references: [Pull Request Review Checklist](https://app.asana.com/0/1202500774821704/1203764234894239/f) [Software Engineering Expectations](https://app.asana.com/0/59792373528535/199064865822552) [Technical Design Template](https://app.asana.com/0/59792373528535/184709971311943) [Pull Request Documentation](https://app.asana.com/0/1202500774821704/1204012835277482/f) --------- Co-authored-by: Elle Sullivan <[email protected]>
Task/Issue URL: https://app.asana.com/0/1148564399326804/1208127166369788/f https://app.asana.com/0/1148564399326804/1208127166369786/f CC: **Description**: PR resolves two edge cases related to automatic updates. Presenting the available update after the update cycle finishes (instead of the update file download completion) Activation of the manual update flow in case the current binary owner is different user Co-authored-by: Dominik Kapusta <[email protected]>
Task/Issue URL: https://app.asana.com/0/1204006570077678/1208002970105121/f Tech Design URL: CC: **Description**: This pull request introduces a change on how we check for valid hostnames. The current regurlar expression had an issue where some mathematical expressions where valid hostnames. BSK PR: duckduckgo/BrowserServicesKit#952 **Steps to test this PR**: 1. Open the app 2. Put 16385-12228.75 in the address bar 3. This should do a search on our SERP instead of trying to open a webpage 4. Test other usual web sites URLs and check those are working. **Definition of Done**: * [x] Does this PR satisfy our [Definition of Done](https://app.asana.com/0/1202500774821704/1207634633537039/f)?
Task/Issue URL: https://app.asana.com/0/1203301625297703/1208137627434482/f **Description**: This change introduces ddg_apple_automation fastlane plugin and its first action, for extracting task ID from Asana task URL. Fastlane setup step was added to all workflows, GHA action calls were replaced by fastlane action calls, outputs were renamed to match action output name and old GHA action was removed.
Task/Issue URL: https://app.asana.com/0/1203301625297703/1208137627434482/f **Description**: This change introduces ddg_apple_automation fastlane plugin and its first action, for extracting task ID from Asana task URL. Fastlane setup step was added to all workflows, GHA action calls were replaced by fastlane action calls, outputs were renamed to match action output name and old GHA action was removed.
Task/Issue URL: https://app.asana.com/0/1206488453854252/1208051900144618/f **Description**: Add macOS Freemium PIR RMF attribute. Temporarily set to hardcoded `false`. [Subsequent task](https://app.asana.com/0/0/1208141497961723/f) will set actual state based on yet-to-be implemented logic.
…3148) Bumps Submodules/privacy-reference-tests from `afb4f61` to `6133e7d`. Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Alessandro Boron <[email protected]>
Task/Issue URL: https://app.asana.com/0/1205922231854923/1207701941801991/f Tech Design URL: CC: **Description**: **Steps to test this PR**: 1. <!-- Tagging instructions If this PR isn't ready to be merged for whatever reason it should be marked with the `DO NOT MERGE` label (particularly if it's a draft) If it's pending Product Review/PFR, please add the `Pending Product Review` label. If at any point it isn't actively being worked on/ready for review/otherwise moving forward (besides the above PR/PFR exception) strongly consider closing it (or not opening it in the first place). If you decide not to close it, make sure it's labelled to make it clear the PRs state and comment with more information. --> **Definition of Done**: * [ ] Does this PR satisfy our [Definition of Done](https://app.asana.com/0/1202500774821704/1207634633537039/f)? --- ###### Internal references: [Pull Request Review Checklist](https://app.asana.com/0/1202500774821704/1203764234894239/f) [Software Engineering Expectations](https://app.asana.com/0/59792373528535/199064865822552) [Technical Design Template](https://app.asana.com/0/59792373528535/184709971311943) [Pull Request Documentation](https://app.asana.com/0/1202500774821704/1204012835277482/f)
Task/Issue URL: https://app.asana.com/0/414709148257752/1207874749913488/f Tech Design URL: CC: Description: This PR updates the app to add WireGuard as a direct dependency, instead of inheriting it from BSK. The issue with the inheritance approach is that WireGuard was being implicitly included in targets that didn't need it, like the VPN agent and app.
…anage Bookmarks cmd+opt+B key equivalent
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Task/Issue URL: https://app.asana.com/0/1202406491309510/1207775978136617/f
Design URL: https://www.figma.com/design/5EpHdIdOlFjdkOy96xUMW3/Bookmark-Management-%F0%9F%93%99?node-id=7410-45232&m=dev
Description:
Steps to test this PR:
Definition of Done:
Internal references:
Pull Request Review Checklist
Software Engineering Expectations
Technical Design Template
Pull Request Documentation