search

duckduckgo / macos-browser

DuckDuckGo macOS Browser
Apache License 2.0
232 stars 7 forks source link

Disable sort and search when no bookmarks #3253

Closed jotaemepereira closed 2 days ago

jotaemepereira commented 1 week ago

Task/Issue URL: https://app.asana.com/0/1204006570077678/1208262956542586/f Tech Design URL: CC:

Description

It disables the sort and search buttons in the bookmarks manager and panel, when the user has no bookmarks.

Acceptance criteria

AC#1 Given a user with no bookmarks, when the user opens the bookmarks panel then the sort and search button are disabled

AC#2 Given a user with no bookmarks, when the user opens the bookmarks manager then the sort and search button are disabled

AC#3 Given a user with no bookmarks, when the user adds a bookmark then the sort and search features are enabled

Demo

https://github.com/user-attachments/assets/8d27720f-b637-4088-95a5-3ade649c3782

Steps to test this PR: Verify the listed acceptance criteria are correct.

Definition of Done:

Internal references:

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

jotaemepereira commented 5 days ago

Looks good overall but I found an issue: the search is still activated by Cmd+F shortcut or by typing a word with empty results; When clearing the search query empty screen is shown instead of the No bookmarks placeholder

Great find @mallexxx. I missed that use case. I’ve added a fix for it, let me know what you think.

mallexxx commented 5 days ago

@jotaemepereira, better, but we need to disable entering the search mode when there‘s no bookmarks: now the bookmarks popover toolbar grows with the search field hidden when typing a letter or hitting Cmd+F; and search activates by itself if a bookmark is added while the popover is shown; I think we shouldn‘t enter the search mode at all while in empty state

jotaemepereira commented 5 days ago

@jotaemepereira, better, but we need to disable entering the search mode when there‘s no bookmarks: now the bookmarks popover toolbar grows with the search field hidden when typing a letter or hitting Cmd+F; and search activates by itself if a bookmark is added while the popover is shown; I think we shouldn‘t enter the search mode at all while in empty state

Thanks. Fixed!

mallexxx commented 3 days ago

@jotaemepereira, I‘ve experimented a bit with key handlers and there were some inconsistencies like Find In Page activated when no bookmarks in the Bookmarks Popover or Esc key not deactivating search in Bookmarks Manager; I ended up adding a Key Equivalent handling view in both the Popover and Manager and opened a PR on top of this PR: https://github.com/duckduckgo/macos-browser/pull/3278