Create ForageVaultWrapper

[dleis612]

What

Add a 3rd VaultWrapper called the ForageVaultWrapper that will be used to accept PINs and interface with the Forage vault.

Why

More information can be found in the ticket.

Demo

ForageVaultWrapper

VGSVaultWrapper

BTVaultWrapper

[dleis612]

@devinmorgan it seems to me that the onFocusChangeListener seems to be working correctly, but I recall we had to add some special handling for this case in the ForagePANEditText class. Do you think this is an issue here too?

[devinmorgan]

Nice work so far, Danny. My understanding is that you said there's still a bit of work you wanted to do in this PR so I'll wait to approve until you take it out of Draft. Cheers!

[devinmorgan]

omg embarrassing - thank you for catching and fixing!

[devinmorgan]

No action required right now but the work we're doing here is surfacing that we probably have some tech debt around the relationship between VaultWrapper and ForagePINEdiText that we'll want to pay down at some point.

That we don't want BTVaultWrapper or VGSVaultWrapper to implement getForageTextElement, indicates that we should not be making getForageTextElement an abstract method on the VaultWrapper class. VaultWrapper already declares getTextElement and getVGSEditText as abstract methods even though they too really should not be living on VaultWrapper since they are specific to BT / VGS wrappers. After poking around the code, it's clear that ForagePINEditText is implemented in a way that expects all vaults expose a getTextElement and getVGSEditText. ForagePINEditTexts dependency on these methods means the fix won't be trivial and so we should probably defer paying this debt down to a later ticket.

One way that I can see to fix this delema is to create an abstraction over the underlying editable text field that BT, VGS, and Forage expose so that there is a unified way of calling things like:

• getUnderlyingView()
• clearText()
• etc

But anyhow, my next step is to make a Linear ticket to capture this and share the link here once I do. But, like a said before, I think what you're doing here is fine and won't break or block anything.

[devinmorgan]

Nice work on this PR Danny! Thanks for your patience with me in the review process. Exciting progress so far!

[devinmorgan]

Nice strategy for separating the PIN validation logic from the PIN rendering logic!

[devinmorgan]

Introducing the data class ParsedStyles definitely made it easier to read the render logic inside ForageVaultWrapper - good idea!

Curious why we put ParsedStyles inside of VaultWrapper instead of keeping it local to ForageVaultWrapper? From what I can tell, this ParsedStyles does not seem to be used by BTVaultWrapper or VGSVaultWrapper and wanted to better understand.

At any rate, where ParsedStyles and it's related logic lives should not hold up merging in this me PR as it's internal code that we can revisit at any point

[devinmorgan]

@devinmorgan it seems to me that the onFocusChangeListener seems to be working correctly, but I recall we had to add some special handling for this case in the ForagePANEditText class. Do you think this is an issue here too?

@dleis612 - really thoughtful concern you bring up here!

You're right that we did some special handling in the past. I suspect that this will not be an issue in our case here because we're using a android.widget.EditText for as the PIN input. The problem in the past seems particular to com.google.android.material.textfield.TextInputEditText, which we were using in the ForagePANEditText.

👆 is just my best guess. In the worst case if we notice a similar problem, I think we can address it then. Lmk whether this is a productive response to your question though