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

Autofill management UI and subdomain matching #2018

Merged
merged 4 commits into from
Jan 11, 2024

Conversation

amddg44
Copy link
Contributor

@amddg44 amddg44 commented Jan 4, 2024

Task/Issue URL: https://app.asana.com/0/1200930669568058/1206195170500835/f
Tech Design URL:
CC:

Description:
Update to autofill management auto selection logic to take subdomains into account + a fix of the highlighting of the selected item

Steps to test this PR:

  1. Manually create a login for https://myaccount.nytimes.com (can be fake credentials)
  2. Visit https://www.nytimes.com/ and open the autofill management UI
  3. Confirm the entry myaccount.nytimes.com is auto selected
  4. Delete any saved credit cards (if any)
  5. Update the filter to select Passwords and confirm the logins list is correctly displayed
  6. Change to select Credit Cards from the dropdown filter and confirm the empty state is correctly displayed

Internal references:

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

@amddg44 amddg44 changed the title Autofill prompt subdomain matching Autofill management UI and subdomain matching Jan 4, 2024
@amddg44 amddg44 requested a review from afterxleep January 10, 2024 14:18
@@ -376,8 +385,6 @@ final class PasswordManagementItemListModel: ObservableObject {
itemsByCategory = itemsByCategory.filter { $0.item(matches: filter) }
}

clearSelection()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I made this change as part of this PR which broke selection highlighting for non empty states so i've moved this call to line 250 where it is now behaving properly for both empty & non-empty states

Copy link
Contributor

@afterxleep afterxleep left a comment

Choose a reason for hiding this comment

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

This looks fine and items are highlighted as expected.

The only thing I've noticed (not related to the changes) is that when I switch to the "Passwords" filter, the current website login is no longer selected by default.

Up to you if you want to review that or add a task for later :)

Screen Recording 2024-01-11 at 5 09 24 PM

@amddg44 amddg44 merged commit db1616e into main Jan 11, 2024
15 checks passed
@amddg44 amddg44 deleted the anya/autofill-prompt-subdomain-matching branch January 11, 2024 16:21
samsymons added a commit that referenced this pull request Jan 11, 2024
* main: (24 commits)
  Autofill management UI and subdomain matching (#2018)
  Update BSK with autofill 10.0.3 (#2051)
  display "you can delete passwords file" for Safari (#2050)
  Bump version to 1.70.0 (101)
  Update embedded files
  Fix permission request (#2046)
  update banner colour (#2044)
  update banner colour (#2044)
  Force reload on DBP notifications (#2028)
  update copy (#2042)
  Suspend Sync when updating device name (#2037)
  update copy (#2042)
  Update Enter Code view in Sync Account Recovery (#2038)
  Suspend Sync when updating device name (#2037)
  Extend hover area for address bar's bookmark button over bookmarks bar and tab bar (#2020)
  Bump version to 1.70.0 (100)
  Update embedded files
  DBP remote messaging (#1997)
  Move Preferences views to SwiftUIExtension package (#2035)
  Add API to obtain reason why given feature is disabled (#2030)
  ...
samsymons added a commit that referenced this pull request Jan 13, 2024
* main:
  Fix serverlocation in connection notification (#2053)
  Use GRDB.swift 2.23.0 (upstream 6.32.0, SQLCipher 4.5.5) (#2021)
  Bump version to 1.70.0 (103)
  Update embedded files
  macOS: Fix issue with not being able to add favorites for existing bookmarks in the latest 1.70.0 internal (#2059)
  Bump version to 1.70.0 (102)
  Update embedded files
  Remove observer from DataBrokerProtectionViewController (#2057)
  Fixes VPN tunnel update logic (#2052)
  Update Sync dialogs titles (#2055)
  Autofill management UI and subdomain matching (#2018)
  Update BSK with autofill 10.0.3 (#2051)
  display "you can delete passwords file" for Safari (#2050)
  Add Bookmark modal in SwiftUI (#2032)
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