Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Address bookmarks feedback #2411

Merged
merged 19 commits into from
Mar 19, 2024
Merged

Conversation

alessandroboron
Copy link
Contributor

Task/Issue URL: https://app.asana.com/0/72649045549333/1206383230169466/f
CC: @samsymons

Description:
This PR addresses feedback around bookmark editing and deleting.
The PR only needs a smoke test because each task has been reviewed incrementally in a sub feature branch. This is just the to merge the main feature branch to main.

Steps to test this PR:
https://app.asana.com/0/0/1206762730363874/f


Internal references:

Pull Request Review Checklist
Software Engineering Expectations
Technical Design Template
Pull Request Documentation

alessandroboron and others added 17 commits March 14, 2024 13:46
Task/Issue URL: https://app.asana.com/0/0/1206531304671947/f

**Description**:
Extracted views to build Bookmark dialog forms in a more composable way.
Task/Issue URL: https://app.asana.com/0/0/1206531304671946/f
CC: @samsymons

**Description**:
1. This PR refactors updates the design for `AddBookmarkPopoverView` and `AddBookmarkFolderPopoverView` and add `AddEditBookmarkFolderView` to add/edit a bookmark folder
Task/Issue URL:
https://app.asana.com/0/72649045549333/1206590790808721/f

**Description**:
1. Add logic and test for `AddEditBookmarkFolderViewModel`.
2. Make a common view to re-use by `AddBookmarkPopoverView` &
`AddEditBookmarkDialogView`
3. Enhance `BookmarkManager` & `BookmarkStore` to update Folder title
and location in one save.
Task/Issue URL:  https://app.asana.com/0/0/1206590790808718/f

**Description**:
1. Implementation of add/edit bookmark dialog logic + tests.
2. Extracted view to be reused by AddBookmarkPopoverView & AddEditBookmarkDialogView
Task/Issue URL: https://app.asana.com/0/0/1206670741563549/f

**Description**:
1. “Three dots hover” menu button for Bookmarks and Folders in the bookmarks shortcut panel.
2. Add “Star" icon for favorite bookmarks.
3. Enhance Contextual Menu with "Edit…” for both folders and bookmarks.
4. Present Add/Edit Bookmarks/Folder dialogs.
Task/Issue URL: https://app.asana.com/0/0/1206687983236915/f

**Description**:
1. Remove Favorites from Manage Bookmarks - Side view
2. Update Bookmarks root folder icon
3. Add “Star” icon to favorites.
4. Remove inline editing for bookmarks
5. Present Add/Edit bookmarks/folders dialog from CTA, Menu buttons and new “Edit…” contextual menu item
Task/Issue URL: https://app.asana.com/0/0/1206713604816890/f

**Description**:
1. Edit Bookmarks & Folders from Bookmarks Bar.
2. Add/Edit Bookmarks from Favorites view.
3. Fix an issue that caused bookmarks to move unnecessarily when already in the root Bookmarks folder
Task/Issue URL: https://app.asana.com/0/0/1206531304671953/f

**Description**:
1. Show / Hide Delete buttons when multiple/single items get selected.
2. Clean up duplicate code for setting up UI and layout.
Task/Issue URL: https://app.asana.com/0/0/1206713604816893/f

**Description**:
1. Centralise contextual menu creation for bookmarks and folders as the menu should be the same no matter where we present it from.
2. Show the same contextual menu for bookmarks from the `Bookmarks Shortcut panel`, `Manage Bookmarks` and `Bookmarks Bar`.
3. Show the same contextual menu for folders from the `Bookmarks Shortcut panel`, `Manage Bookmarks` and `Bookmarks Bar`.
4. Add “Open All in New Window” to the folder menu.
5. Add “Add Folder”, “Manage Bookmarks” and “Move to End” to all the menus.
Task/Issue URL: https://app.asana.com/0/0/1206687983236914/f

**Description**:
Fix an issue where the Bookmark panel accessible via the Bookmarks shortcut wasn’t reloading when updating the Bookmark URL.
Task/Issue URL: https://app.asana.com/0/0/1206772004955801/f

**Description**:
Remove old Bookmarks Views and View Models not used anymore.
Task/Issue URL: https://app.asana.com/0/0/1206762730363882/f

**Description**:
When adding a bookmark from the bookmark receipt the buttons were showing in the wrong size.
Task/Issue URL: https://app.asana.com/0/0/1206762730363836/f

**Description**:
I found an issue while doing a smoke test for the Ship Review. The first time a bookmark/folder is added from the bookmarks shortcut panel and we open the menu from the “…” menu button, the menu is disabled. The same
is not true if we open the menu by right-clicking on the item.
#2354)

Task/Issue URL: https://app.asana.com/0/0/1206762730363884/f

**Description**:
In some cases `BookmarkFolder`s that are added to the root folder have `parentFolderUUID` equal to `nil` in some cases they have it equal to the root identifier `bookmarks_root`. Ensure that equality is satisfied when this happens.
This bug started when we changed hashable/equatable.
@github-actions github-actions bot added the bot: not in app board Added by automation for pull requests with tasks not added to macOS App Board Asana project label Mar 14, 2024
Copy link
Contributor

github-actions bot commented Mar 14, 2024

Warnings
⚠️ PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 🚫 dangerJS against 1ce025a

@alessandroboron alessandroboron removed the bot: not in app board Added by automation for pull requests with tasks not added to macOS App Board Asana project label Mar 14, 2024
Task/Issue URL: https://app.asana.com/0/0/1206828077023778/f

**Description**:
Update chevron icon for Manage Bookmarks
Copy link
Collaborator

@SabrinaTardio SabrinaTardio left a comment

Choose a reason for hiding this comment

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

Great Job!!!
The code was all reviewed before.
I tested all scenario and it all works, just two things:

  • I think Scenario 4 - New Bookmark CTA - Selected Folder from Right Side View in the manager is wrong and the behaviour is correct, just ask to confirm
  • If you try to move a folder in itself there is a error and assertion failure I think ideally we don’t send the request to the store or not show the folder

Copy link
Collaborator

@SabrinaTardio SabrinaTardio left a comment

Choose a reason for hiding this comment

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

Great Job!!! 👏

Task/Issue URL: https://app.asana.com/0/0/1206858401182957/f

**Description**:
Fix the chevron icon on Manage Bookmarks
@alessandroboron alessandroboron merged commit 7856a44 into main Mar 19, 2024
20 checks passed
@alessandroboron alessandroboron deleted the alessandro/address-bookmarks-feedback branch March 19, 2024 08:13
samsymons added a commit that referenced this pull request Mar 19, 2024
* main:
  Display error messaging for cancelled subscriptions (#2394)
  Updated Settings Page (#2329)
  Add VPN & PIR thank you modal (#2437)
  Web pixels (handlers + pixels) (#2451)
  Subscription design review further fixes (#2448)
  Address bookmarks feedback (#2411)
  Handle subscription-related iOS use cases (#2427)
samsymons added a commit that referenced this pull request Mar 20, 2024
# By Michal Smaga (2) and others
# Via GitHub (2) and Dominik Kapusta (1)
* main:
  Connect thank you message to DBP method (#2456)
  Use SubscriptionFeatureAvailability to determine availability of the subscription (#2436)
  Add Privacy Pro to App Store build (#2440)
  Display error messaging for cancelled subscriptions (#2394)
  Updated Settings Page (#2329)
  Add VPN & PIR thank you modal (#2437)
  Web pixels (handlers + pixels) (#2451)
  Subscription design review further fixes (#2448)
  Address bookmarks feedback (#2411)
  Handle subscription-related iOS use cases (#2427)
  Remove hardcoded NetP staging endpoint (#2446)
  DBP: Make webview non-persistent and delete any old cache data (#2445)
  Prevents the tunnel from starting without an auth token (#2438)
  Use History in Suggestions on iOS (#2339)
  When publishing a DMG, only check out the branch if it exists, otherwise stay on main
  Bump version to 1.80.0 (146)
  Update autoconsent to v10.3.0 (#2433)

# Conflicts:
#	DuckDuckGo.xcodeproj/project.pbxproj
#	DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
samsymons added a commit that referenced this pull request Mar 21, 2024
# By Diego Rey Mendez (4) and others
# Via GitHub (2) and Dominik Kapusta (1)
* main: (28 commits)
  Entitlements cache fixes (#2465)
  BSK update for RMF app build version improvement (#2362)
  Surface the last-used password first in autofill prompt when filling passwords (#2431)
  Reset subs VPN state from debug menu  (#2462)
  iOS UI improvements (#2412)
  Updates VPN 128 assets (#2459)
  Fix sparkle strings for thank you message (#2461)
  Wire VPN environment to subs menu item  (#2460)
  Makes a method thread safe (#2452)
  Ensure smooth subs updates (#2424)
  Update Pointfree snapshot testing library to 1.15.4 (#2449)
  Connect thank you message to DBP method (#2456)
  Use SubscriptionFeatureAvailability to determine availability of the subscription (#2436)
  Add Privacy Pro to App Store build (#2440)
  Display error messaging for cancelled subscriptions (#2394)
  Updated Settings Page (#2329)
  Add VPN & PIR thank you modal (#2437)
  Web pixels (handlers + pixels) (#2451)
  Subscription design review further fixes (#2448)
  Address bookmarks feedback (#2411)
  ...

# Conflicts:
#	DuckDuckGo.xcodeproj/project.pbxproj
#	DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
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.

2 participants