[FX-1278] Remove ability to set hint on ForagePINEditText
#244
Conversation
We only ever used XML to set the PIN hint text and color in the Sample app. We never used the programmatic `setHint(...)` or `setHintTextColor(...)` Signed-off-by: Devin Morgan <[email protected]>
Forage wants it's merchants to adopt the best practice of not having any placeholder text inside ForagePINEditText elements for the reasons enumerated in this slack message: https://joinforage.slack.com/archives/C056QGRF6S3/p1712687133981029?thread_ts=1712680102.191409&cid=C056QGRF6S3 Previously hints for PINs could be set via XML and at runtime. This commit removes ForageSDK's ability to set hint parameters via XML. Signed-off-by: Devin Morgan <[email protected]>
The sequel to the previous commit: this commit removes ForageSDK's ability to set hint parameters at runtime Signed-off-by: Devin Morgan <[email protected]>
|
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @devinmorgan and the rest of your teammates on |
devinmorgan
changed the title
hint on ForagePINEditText
devinmorgan
requested review from
owenkim,
ForageRob,
dleis612,
djoksimo and
evanfreeze
as code owners
There was a problem hiding this comment.
We can remove hint and hindTextColor from the PIN-specific code without using @deprecated if we know that our existing merchants are setting (which they are not AFAIK)
However, we should be cautious about how we handle this change for the ForageElement/PAN element
We still want the merchant to be able to set the placeholder on PAN elements.
Would this be a breaking change for clients who may be setting the placeholder on the panEditText?
| /** | ||
| * Sets the text to be displayed when the ForageElement input field is empty. | ||
| * | ||
| * @param hint The text to display. | ||
| */ | ||
| fun setHint(hint: String) | ||
|
|
||
| /** | ||
| * Sets the hint text color. | ||
| * | ||
| * @param hintTextColor The color value in the form `0xAARRGGBB`. | ||
| */ | ||
| fun setHintTextColor(hintTextColor: Int) |
There was a problem hiding this comment.
We still want the merchant to be able to set the placeholder on PAN elements.
Would this be a breaking change for clients who may be setting the placeholder on the panEditText?
There was a problem hiding this comment.
Question: Is Gopuff using this and setting the value to the empty string currently?
| override fun setHint(hint: String) { | ||
| _editText.hint = hint | ||
| } | ||
|
|
||
| override fun setHintTextColor(hintTextColor: Int) { | ||
| _editText.setHintTextColor(hintTextColor) | ||
| } |
There was a problem hiding this comment.
instead of removing this altogether ; can we use @Deprecated / @deprecated with level hidden / warning?
https://kotlinlang.org/api/latest/jvm/stdlib/kotlin/-deprecation-level/
|
We are opting to depreciate instead of delete. Closing this in favor of #271 |
What
Removes
ForagePINEditText's ability to set a hint or hint color both at runtime (programmatically) and at build time (XML)Why
Forage wants to ensure that all SDK users can follow recommended best practices for EBT payments as easily as possible. Hints should not exist for PINs and this PR aims to enable merchants to follow hint-related best practices without confusion
Test Plan
image of hint-less PIN field
How
This change alters the behavior of the SDK in a way that merchants would be able to feel if they were not following best practices. If we are concerned that merchants may be using hints for PINs, then a strategy for a thoughtful release should be discussed before merging this PR. However, if we suspect the risk is low, then this PR can be merged as is