[ABW-3959] Factor Source Card component #1402
Conversation
RadixWallet/Core/Resources/Resources/en.lproj/Localizable.strings
Outdated
Show resolved
Hide resolved
| @@ -1792,6 +1792,52 @@ internal enum L10n { | |||
| } | |||
| } | |||
| internal enum FactorSources { | |||
| internal enum Card { | |||
There was a problem hiding this comment.
guessing we don't have them on localisation repo yet due to issues with integration?
| // MARK: - WarningErrorView | ||
| struct WarningErrorView: View { | ||
| // MARK: - StatusMessageView | ||
| struct StatusMessageView: View { |
| @@ -0,0 +1,77 @@ | |||
| // MARK: - FactorSourceCardDataSource | |||
| struct FactorSourceCardDataSource { | |||
There was a problem hiding this comment.
really nice that you followed AccountCard approach
| onRemoveTapped: onRemoveTapped | ||
| ) | ||
| case let .instanceRegular(factorSource): | ||
| guard let details = factorSource.factorSourceKind.details else { return nil } |
There was a problem hiding this comment.
as mentioned on demo, I don't think the validation of which cards can be built should be made based on details, icon or title available. Instead, we would have a var isSupported: Bool or similar done at first, and have icon, title and details always return required values.
Once that change is done, the only difference between instanceCompact & instanceRegular would be showing a subtitle or not. I would then recommend one of the two following solutions:
enum Kind {
case genericDescription(FactorSourceKind)
case instance(factorSource: FactorSource, showDetails: Bool)
case instanceLastUsed(
factorSource: FactorSource,
accounts: [Account] = [],
personas: [Persona] = []
)
}or
enum Kind {
case genericDescription(FactorSourceKind)
case instance(factorSource: FactorSource, kind: InstanceKind)
enum InstanceKind {
case short(showDetails: Bool)
case extended(accounts: [Account] = [], personas: [Persona])
}
}I like this last idea better because, as you see on the switch that iterate over the Kind, the instance cases are mostly repeated.
| case let .instanceCompact(factorSource), | ||
| let .instanceRegular(factorSource), | ||
| let .instanceLastUsed(factorSource, _, _): | ||
| factorSource.factorSourceKind |
There was a problem hiding this comment.
this would change to
| case let .instanceCompact(factorSource), | |
| let .instanceRegular(factorSource), | |
| let .instanceLastUsed(factorSource, _, _): | |
| factorSource.factorSourceKind | |
| case let .instance(factorSource, _) | |
| factorSource.factorSourceKind |
| var accounts: [Account] = [] | ||
| var personas: [Persona] = [] | ||
|
|
||
| var hasAccountsOrPersonas: Bool { |
There was a problem hiding this comment.
I would name this hasEntities or hasAnyAccountsOrPersonas
(because hasAccountsOrPersonas sounds like something which would return an enum)
There was a problem hiding this comment.
hasEntities sounds good
| @@ -96,7 +96,15 @@ extension FactorSource { | |||
| lastUsedOn: .now, | |||
| flags: [] | |||
| ), | |||
| hint: DeviceFactorSourceHint(name: name, model: "iPhone", mnemonicWordCount: .twentyFour, systemVersion: nil, hostAppVersion: nil, hostVendor: nil) | |||
| hint: DeviceFactorSourceHint( | |||
There was a problem hiding this comment.
I think there should be a .sampleValue ?
There was a problem hiding this comment.
No, DeviceFactorSourceHint does not have sample values exposed to hosts
| } | ||
|
|
||
| extension FactorSourceKind { | ||
| var icon: ImageResource? { |
There was a problem hiding this comment.
this is optional because we don't have examples of icon for .trustedContact, .securityQuestions? Or because these should not have an icon?
Same for the rest, I would put a fatal error instead of returning nil, if the reason is just that we miss icon and so on.
|
|
||
| switch kind { | ||
| case let .genericDescription(factorSourceKind): | ||
| guard let details = factorSourceKind.details else { return nil } |
There was a problem hiding this comment.
a factorSourceKind does not have details as per requirements or that we don't have these yet?
There was a problem hiding this comment.
made details not optional
Jira ticket: ABW-3959
Description
Adds reusable and configurable factor source card component
Video
Simulator.Screen.Recording.-.iPhone.16.-.2024-11-29.at.15.37.24.mp4