-
Notifications
You must be signed in to change notification settings - Fork 30
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
Swap to using cipher list view #1124
base: main
Are you sure you want to change the base?
Conversation
searchResults: searchText != nil ? ciphers : nil | ||
) { | ||
sections.append(fido2Section) | ||
if #available(iOSApplicationExtension 17.0, *) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: Need to figure out how the fido2 flow would work. @coroiu do we need to fully decrypt items, or could we expose the needed details in CipherListView
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fido2 credentials are stored as encrypted inside CipherView
so you don't need to decrypt those. But you need some Cipher data to be able to create the Fido2AutofillCredentialView
, like cipher.login.username
and cipher.name
}) | ||
} | ||
} | ||
// // Add any additional actions for the type of cipher selected. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: We probably need to adjust this to be lazy vs eager, and ad-hoc decrypt items when clicking on items.
No New Or Fixed Issues Found |
BitwardenShared/UI/Vault/Helpers/VaultItemMoreOptionsHelper.swift
Outdated
Show resolved
Hide resolved
// let decryptedFido2Credentials = try await clientService | ||
// .platform() | ||
// .fido2() | ||
// .decryptFido2AutofillCredentials(cipherView: cipher) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Hinton it looks like we'll need to have decryptFido2AutofillCredentials(cipherView:)
updated to accept a CipherListView
here unless I am missing something.
🎟️ Tracking
📔 Objective
We should default to using
CipherListView
instead ofCipherView
for vault lists, and only decryptCipherView
when we need to display the full items.📸 Screenshots
⏰ Reminders before review
🦮 Reviewer guidelines
:+1:
) or similar for great changes:memo:
) or ℹ️ (:information_source:
) for notes or general info:question:
) for questions:thinking:
) or 💭 (:thought_balloon:
) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion:art:
) for suggestions / improvements:x:
) or:warning:
) for more significant problems or concerns needing attention:seedling:
) or ♻️ (:recycle:
) for future improvements or indications of technical debt:pick:
) for minor or nitpick changes