[FX-1460] Insert ForageVaultElement into PIN class hierarchy #267

devinmorgan · 2024-06-13T17:29:57Z

What (and Why)

ForageVaultElement is basically the union type ForagePinElement and ForagePinPad (pos repo). To realize this union type, we need to ensure that ForagePinElement subclasses ForageVaultElement.

The main complexity of this initiative came from ForageVaultElement being a generic class, so it requires knowing the shape of the state passed to on-change events, focus / blur, etc. We now have (or rather, need) three different states:

PanEditTextState
PinEditTextState
PinPadState (pos repo only)

The problem was that PinElementStateManager was not flexible enough to support a PinEditTextState and a PinPadState, and this meant we needed to re-home the state-tracking functionality into more atomic concepts such as FocusState and PinInputState; teasing out these separate concerns now gives us the flexibility we needed to support all three new states outlined above

Why

See above

Test Plan

✅ I've modified the tests suites as part of this PR.
❌ The reviewer does not need to manually test the changes in this PR; passing CI tests is sufficient.

Demo

N/A since it's just an implementation change

How

Can be rolled out once #266 is merged

devinmorgan · 2024-06-13T17:30:13Z

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @devinmorgan and the rest of your teammates on Graphite

...e-android/src/main/java/com/joinforage/forage/android/core/ui/element/state/EditTextState.kt

...ge-android/src/main/java/com/joinforage/forage/android/core/ui/element/state/ElementState.kt

djoksimo · 2024-06-14T21:29:27Z

...id/src/main/java/com/joinforage/forage/android/core/ui/element/state/pan/PanEditTextState.kt

+interface DerivedCardInfo {
+    val usState: USState?
+}
+
+/**
+ * An interface that represents the state of a
+ * [ForagePANEditText][com.joinforage.forage.android.ui.ForagePANEditText] Element.
+ * @property derivedCardInfo The [DerivedCardInfo] for the card number submitted to the Element.
+ */
+interface PanEditTextState : EditTextState {
+    val derivedCardInfo: DerivedCardInfo // the interface not the DTO


Are both the interface and the dto supposed to be public?

Great question! Two notes here:

Regarding the interface, PanEditTextState, being public
Since this interface get's exposed in a few methods in ForagePanElement, we unfortunately need to make this interface public.

Regarding the dto being public
What I think may be relevant to point out is that we no longer have a data class that implements the PanEditTextState; instead PanEditTextState.from(...) (see below) returns an anonymous object. Returning an anonymous object that implements the PanEditTextState instead of defining a specific data class has the desirable affect of us not needing to add any DTO classes to the public surface area! But, lmk I am not speaking to your point as I could by misunderstanding.

...id/src/main/java/com/joinforage/forage/android/core/ui/element/state/pin/PinEditTextState.kt

devinmorgan · 2024-06-17T12:18:48Z

forage-android/src/main/java/com/joinforage/forage/android/core/ui/element/ForagePanElement.kt

    override fun setOnFocusEventListener(l: SimpleElementListener) {
-        manager.setOnFocusEventListener(l)
+        onFocusEventListener = l
        restartFocusChangeListener()
    }
    override fun setOnBlurEventListener(l: SimpleElementListener) {
-        manager.setOnBlurEventListener(l)
+        onBlurEventListener = l
        restartFocusChangeListener()
    }
-    override fun setOnChangeEventListener(l: StatefulElementListener<PanElementState>) {
-        manager.setOnChangeEventListener(l)
+    override fun setOnChangeEventListener(l: StatefulElementListener<PanEditTextState>) {
+        onChangeEventListener = l
    }

-    override fun getElementState(): PanElementState {
-        return manager.getState()
-    }
+    override fun getElementState(): PanEditTextState = PanEditTextState.from(
+        focusState,
+        inputState
+    )


These methods expose the PanElementState to the public surface area, which forces us to make PanElementState public 😕

ForageVaultElement is basically the union type ForagePinElement and ForagePinPad (pos repo). To realize this union type, we need to ensure that ForagePinElement subclasses ForageVaultElement. The main complexity of this initiative came from ForageVaultElement being a generic that requires knowing the shape of the state that will be passed in on-change events, focus / blur, etc. We now have (or rather, need) three different states: 1. PanEditTextState 2. PinEditTextState 3. PinPadState (pos repo only) The problem was that PinElementStateManager was not flexible enough to support a PinEditTextState and a PinPadState, and this meant we needed to re-home the state tracking functionality into more atomic concepts such as FocusState and PinInputState; teasing out these separate concerns gave us the the flexibility we needed to support all three new states outlined above Signed-off-by: Devin Morgan <[email protected]>

devinmorgan · 2024-06-17T15:45:30Z

Merge activity

Jun 17, 11:45 AM EDT: Graphite rebased this pull request after merging its parent, because this pull request is set to merge when ready.
Jun 17, 11:49 AM EDT: @devinmorgan merged this pull request with Graphite.

devinmorgan changed the title ~~Insert ForageVaultElement into PIN class hierarchy~~ [FX-1459] Insert ForageVaultElement into PIN class hierarchy Jun 13, 2024

devinmorgan marked this pull request as ready for review June 13, 2024 20:30

devinmorgan requested review from owenkim, ForageRob, dleis612, djoksimo and evanfreeze as code owners June 13, 2024 20:30

devinmorgan changed the title ~~[FX-1459] Insert ForageVaultElement into PIN class hierarchy~~ [FX-1460] Insert ForageVaultElement into PIN class hierarchy Jun 13, 2024

djoksimo approved these changes Jun 14, 2024

View reviewed changes

djoksimo requested changes Jun 14, 2024

View reviewed changes

devinmorgan force-pushed the devin/fx-1459-sync-edittextelement-and-forageelementt-interfaces-with-poss branch from 10a0d66 to 2dec633 Compare June 17, 2024 12:02

devinmorgan force-pushed the devin/fx-1460-introduce-the-foragevaultelement-into-the-foragepinelement branch from cb896ab to 4042887 Compare June 17, 2024 12:02

devinmorgan force-pushed the devin/fx-1459-sync-edittextelement-and-forageelementt-interfaces-with-poss branch from 2dec633 to d197e36 Compare June 17, 2024 12:11

devinmorgan force-pushed the devin/fx-1460-introduce-the-foragevaultelement-into-the-foragepinelement branch from a62d07a to cf140c8 Compare June 17, 2024 12:11

devinmorgan commented Jun 17, 2024

View reviewed changes

djoksimo approved these changes Jun 17, 2024

View reviewed changes

devinmorgan force-pushed the devin/fx-1459-sync-edittextelement-and-forageelementt-interfaces-with-poss branch from d197e36 to 9fc0eff Compare June 17, 2024 15:38

devinmorgan changed the base branch from devin/fx-1459-sync-edittextelement-and-forageelementt-interfaces-with-poss to main June 17, 2024 15:41

devinmorgan and others added 2 commits June 17, 2024 15:44

Add missing docstrings

dccef8f

devinmorgan force-pushed the devin/fx-1460-introduce-the-foragevaultelement-into-the-foragepinelement branch from cf140c8 to dccef8f Compare June 17, 2024 15:44

devinmorgan merged commit 4a9d818 into main Jun 17, 2024
4 of 6 checks passed

devinmorgan deleted the devin/fx-1460-introduce-the-foragevaultelement-into-the-foragepinelement branch July 1, 2024 12:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FX-1460] Insert ForageVaultElement into PIN class hierarchy #267

[FX-1460] Insert ForageVaultElement into PIN class hierarchy #267

devinmorgan commented Jun 13, 2024 •

edited

Loading

devinmorgan commented Jun 13, 2024 •

edited

Loading

djoksimo Jun 14, 2024

devinmorgan Jun 17, 2024

devinmorgan Jun 17, 2024

devinmorgan commented Jun 17, 2024 •

edited

Loading

[FX-1460] Insert ForageVaultElement into PIN class hierarchy #267

[FX-1460] Insert ForageVaultElement into PIN class hierarchy #267

Conversation

devinmorgan commented Jun 13, 2024 • edited Loading

What (and Why)

Why

Test Plan

Demo

How

devinmorgan commented Jun 13, 2024 • edited Loading

djoksimo Jun 14, 2024

Choose a reason for hiding this comment

devinmorgan Jun 17, 2024

Choose a reason for hiding this comment

devinmorgan Jun 17, 2024

Choose a reason for hiding this comment

devinmorgan commented Jun 17, 2024 • edited Loading

Merge activity

devinmorgan commented Jun 13, 2024 •

edited

Loading

devinmorgan commented Jun 13, 2024 •

edited

Loading

devinmorgan commented Jun 17, 2024 •

edited

Loading