Password manager: Fix tabbing between text fields

[noisysocks]

Task/Issue URL: #3723

Description:

Fix tabbing between text fields in the password manager by manually recalculating key view loop in PasswordManagementViewController. This isn't done automatically as MainWindow disables autorecalculatesKeyViewLoop.

Optional E2E tests:

• Run PIR E2E tests
  Check this to run the Personal Information Removal end to end tests. If updating CCF, or any PIR related code, tick this.

Steps to test this PR:

1. Click Open application menu (⋯)
2. Click Passwords & Autofill
3. Click Add item (+)
4. Select any type of item, e.g. Password
5. Focus on a text field, e.g. the title field
6. Press Tab
7. Focus should move to the next text field.

Definition of Done:

• Does this PR satisfy our Definition of Done?

---

Internal references:

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

[amddg44]

Nice work @noisysocks, this change works perfectly and is a great UX improvement, thank you!

I did notice that the call to recalculateKeyViewLoop() triggers every time an item is selected, not just when adding a new item but from a performance perspective it doesn’t appear to have an impact so should be safe to leave as is 🙂

[amddg44]

If you feel inspired to also tackle tabbing between text fields when editing, I think the same fix should work if applied within this code block (around line 150 of this file) when isEditing == true, but totally up to you!

            editingCancellable = itemModel?.isEditingPublisher.sink(receiveValue: { [weak self] isEditing in
                guard let self = self else { return }

                self.isEditing = isEditing
                self.divider.isHidden = isEditing
                self.updateEmptyState(state: self.listModel?.emptyState)

                self.searchField.isEditable = !isEditing
            })

[noisysocks]

I did notice that the call to recalculateKeyViewLoop() triggers every time an item is selected, not just when adding a new item but from a performance perspective it doesn’t appear to have an impact so should be safe to leave as is 🙂

Yeah cool. I think conceptually it makes sense to recalculate the key view loop whenever a new item is selected since the new child view might (one day) have a different set of fields that can be tabbed between.

If you feel inspired to also tackle tabbing between text fields when editing, I think the same fix should work if applied within this code block (around line 150 of this file) when isEditing == true, but totally up to you!

I’ll play with this!

[noisysocks]

Hey @amddg44. What do you think of the alternative approach I just pushed up?

Instead of having PasswordManagementViewController call recalculateKeyViewLoop() I’ve added a NSHostingView subclass which is responsible for manually recalculating key view loops after SwiftUI finishes layout. The intention is you’d use this subclass instead of NSHostingView wherever you’re hosting a SwiftUI view that has text fields or anything that relies on default focus behaviour.

Doing it this way means the bug is fixed in more cases (including the one you spotted) and also lets me remove the call to DispatchQueue.main.async which I think was a bit hacky.

Pretty rusty with macOS/iOS development so please let me know if I’m off piste here!

[amddg44]

Hey @noisysocks, I like this alternate approach! However hooking in on layout() looks to be a bit a too aggressive; I set up a logging breakpoint and if you scroll at all while in add / edit mode there is an immediate flood 20+ layout calls. Here is what I logged just briefly scrolling down the form when creating a new entry 🙂

Some options from here could be to simply add in a throttling mechanism within the layout() function. Alternatively perhaps triggering just on viewDidMoveToWindow() could be a good compromise, although I believe this approach would also required an additional function in AutoRecalculatingKeyViewHostingView to be added to trigger recalculateKeyViewLoop, that would be called when add or edit is clicked.

[amddg44]

Oh and one minor note - can you please include any changes to project.pbxproj so Xcode can pick up the new file? Thanks!