Address bar refactoring #1610
Address bar refactoring #1610
Conversation
| override func paste(_ sender: Any?) { | ||
| // Fixes an issue when url-name instead of url is pasted | ||
| if let url = NSPasteboard.general.url { | ||
| super.pasteAsPlainText(url.absoluteString) |
There was a problem hiding this comment.
removed extra controlTextDidChange call: paste/pasteAsPlainText already calls controlTextDidChange
| @available(macOS 12.0, *) | ||
| override var textLayoutManager: NSTextLayoutManager? { | ||
| if let textLayoutManager = super.textLayoutManager, | ||
| !(textLayoutManager.textSelectionNavigation is AddressBarTextSelectionNavigation) { |
There was a problem hiding this comment.
from macOS 12 TextKit uses new architecture, need to override TextSelectionNavigation to adjust by-word text selection
|
|
||
| // MARK: - Exclude “ – Search with DuckDuckGo” suffix from selection range | ||
|
|
||
| override func setSelectedRanges(_ ranges: [NSValue], affinity: NSSelectionAffinity, stillSelecting stillSelectingFlag: Bool) { |
There was a problem hiding this comment.
final editor selection adjustment. disables putting cursor in the Suffix
| breakUndoCoalescingIfNeeded(for: InputType(string)) | ||
|
|
||
| addressBar.textView(self, userTypedString: string, at: replacementRange.location == NSNotFound ? self.selectedRange() : replacementRange) { | ||
| super.insertText(string, replacementRange: replacementRange) |
There was a problem hiding this comment.
see comment in textView:userTypedString:at: - using callback to set temporary flags for handleTextDidChange
|
|
||
| // MARK: - Moving selection by word | ||
|
|
||
| @available(macOS, deprecated: 12.0, message: "Move this logic to AddressBarTextSelectionNavigation") |
There was a problem hiding this comment.
leaving this logic here until we drop Big Sur
|
|
||
| override var allowsUndo: Bool { | ||
| get { | ||
| !(addressBar?.isUndoDisabled ?? false) && super.allowsUndo |
There was a problem hiding this comment.
disable adding undo action when the text field is set directly e.g. for suggestion autocompletion
| defer { | ||
| lastInputType = inputType | ||
| } | ||
| switch (lastInputType, inputType) { |
There was a problem hiding this comment.
"merges" undo actions of the same type into coalesced actions. When entering separators the coalescing is broken to avoid undoing the whole user input but only undo the last typed word
| import Combine | ||
| import Common | ||
| import BrowserServicesKit | ||
|
|
||
| protocol AddressBarTextFieldDelegate: AnyObject { | ||
|
|
||
| func adressBarTextField(_ addressBarTextField: AddressBarTextField, didChangeValue value: AddressBarTextField.Value) |
There was a problem hiding this comment.
value is already published so removed this to avoid using 2 patterns and only observe the published property
There was a problem hiding this comment.
Wow this is a great improvement! 🏅 The user experience of the address bar is so much better now.
In terms of code, I don't have any comment. The logic is pretty complicated so the codebase just reflects that. I am missing some unit tests, but since you work with subclasses of views, I don't think it's even possible to reasonable unit test anything here.
I only have one concert but it doesn't have to be correct. Do you feel like performance of address bar is lower with this change? For example, when typing, it feels slower. If not, then feel free to merge! LGTM!
… context menu adjustments
|
@tomasstrba, I‘ve profiled text editing - nothing really stood out to me except Sharing Menu updating on each hotkey press - which eats some time but it‘s not a regression and to be handled in a separate PR. TextView inner machinery takes much more CPU time than all our code. |
# By Alexey Martemyanov (3) and Dominik Kapusta (1) # Via GitHub * develop: disable flaky testWhenAutoconsentDisabled_promptIsDisplayed (#1619) Prevent crash on database init failure (#1617) Address bar refactoring (#1610) Switch Sync environment to production (#1622) # Conflicts: # DuckDuckGo/AppDelegate/AppDelegate.swift
# By Alexey Martemyanov (8) and others # Via GitHub * develop: Clean up login titles at insert and update + migration (#1605) Changes for DBP internal release (#1621) Fix NetP builds in the DBP target (#1627) Put Sparkle behind a new SPARKLE flag (#1616) prevent WKFullScreenWindowController crashing after `close` call (#1618) Use Development environment for Sync by default in Debug builds (#1625) disable flaky testWhenAutoconsentDisabled_promptIsDisplayed (#1619) Prevent crash on database init failure (#1617) Address bar refactoring (#1610) Switch Sync environment to production (#1622) Fixes startup options reset (#1581) perform Email Keychain migration only once (#1600) Fix NetP embedding in DBP builds (#1614) Bump version to 1.56.0 (58) Update embedded files fix Package.resolved New deployment target set to 11.4 (#1592) # Conflicts: # DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved # DuckDuckGo/Main/Main.swift
# By Alexey Martemyanov (7) and others * release/1.57.0: (24 commits) Bump version to 1.57.0 (59) Update embedded files Remove sandbox entitlement from NetP login item (#1629) Make sure Catalina build is in the appcast (#1632) implement onboarding experiment (#1575) fix bug where when in case of error address shown address bar is wrong (#1604) Update BSK for iOS phased rollout feature flag (#1623) Autofill update for iOS de-duplication of prompted logins (#1612) Tool for uploading release files to s3 (#1561) fix Safari import not working (#1630) Refactor Sharing menu, fix QLPreviewPanel warnings (#1624) Clean up login titles at insert and update + migration (#1605) Changes for DBP internal release (#1621) Fix NetP builds in the DBP target (#1627) Put Sparkle behind a new SPARKLE flag (#1616) prevent WKFullScreenWindowController crashing after `close` call (#1618) Use Development environment for Sync by default in Debug builds (#1625) disable flaky testWhenAutoconsentDisabled_promptIsDisplayed (#1619) Prevent crash on database init failure (#1617) Address bar refactoring (#1610) ... # Conflicts: # DuckDuckGo/ContentBlocker/macos-config.json
Task/Issue URL: https://app.asana.com/0/1177771139624306/1205421610519119/f
BSK PR: duckduckgo/BrowserServicesKit#499
iOS PR: duckduckgo/iOS#1989
Description:
Steps to test this PR:
Internal references:
Pull Request Review Checklist
Software Engineering Expectations
Technical Design Template
Pull Request Documentation