Address Bar/Search Experience improvements #103
Conversation
| let suggestions = suggestions?.map { suggestion -> Suggestion in | ||
| if case .phrase(phrase: let phrase) = suggestion, | ||
| let url = phrase.punycodedUrl, url.isValid { | ||
| return .website(url: url) |
There was a problem hiding this comment.
I moved the code converting a suggestion to URL here to make it the single source of truth located in the Model layer
There was a problem hiding this comment.
I suggest to move it into Suggestion.swift
There was a problem hiding this comment.
I'm not sure about this one as it would require also bringing the Punycode package dependency into BrowserServicesKit, @samsymons what do you think?
| suggestionContainer.getSuggestions(for: userStringValue) | ||
| private(set) var userStringValue: String? | ||
|
|
||
| private var userTyped = false |
There was a problem hiding this comment.
Please, is there a more descriptive name for this property? The best is when a reader doesn’t need to look at the algorithm to guess what is the purpose of it
| let suggestions = suggestions?.map { suggestion -> Suggestion in | ||
| if case .phrase(phrase: let phrase) = suggestion, | ||
| let url = phrase.punycodedUrl, url.isValid { | ||
| return .website(url: url) |
There was a problem hiding this comment.
I suggest to move it into Suggestion.swift
| return suggestion | ||
| } | ||
| .enumerated() | ||
| .sorted { lhs, rhs -> Bool in |
There was a problem hiding this comment.
Few things here:
- Ordering is responsibility of the shared algorithm.
- Please, just don’t bother with the ordering of suggestions in this task at all. I’m currently kicking of a project that will add history items and cover ordering. I’ll make the tech design and a product feedback request so we get some feedback before the implementation. (Below, putting websites before phrases is probably not right. The order was set by the remote service and preferring a website before a phrase all the time could make searching experience pretty weird + less profitable for us as a company)
|
Alex, thanks for edits, works great now! 💯 Do you think we could smooth the experience a bit? Every time user types a new letter, we basically deselect the row and select the row again even when it could stay selected. It makes the experience too blinking I think and it could be fixed pretty easily, for example cache selected row until new suggestions are delivered (but only when it makes sense). Please look at attachment if its not clear what I’m talking about.
|
…eskit-url' into feature/alex/address-bar
|
@tomasstrba I did the changes we discussed:
|
…eskit-url' into feature/alex/address-bar Restore Email Beta generateTokenPageURL
Task/Issue URL: https://app.asana.com/0/1203137811378537/1204667932873181/f CC: @samsymons Description: Follow-for to Restore the Developer ID script #103 Set Deploy location for Developer ID build to /Applications/DEBUG Added extra NetP Debug menu items to uninstall SysEx/Login Items Moved NetP debug menu items handling methods to AppDelegate to allow their usage when no windows are open
Task/Issue URL: https://app.asana.com/0/1199237043630360/1200180252402997
Design URL: https://www.figma.com/file/GMASm53B24PF9R0N8MpoDi/Bookmarks-in-Search-Suggestions?node-id=1%3A17757
CC: @tomasstrba @samsymons @brindy
Description:
This PR contains the following changes:
Steps to test this PR:
Internal references:
Software Engineering Expectations
Technical Design Template
When ready for review, remember to post the PR in MM