ChaosLeung / PinView

A PIN view library for Android. Use to enter PIN/OTP/password etc.
Apache License 2.0
634 stars 121 forks source link

add programmatical way to reveal or hide the password #54

Closed alexcohn closed 4 years ago

alexcohn commented 4 years ago

added public void setIsPassword()

note that the legacy behavior is preserved: change of inputType will also hide or reveal the password, as before.

answers https://github.com/ChaosLeung/PinView/issues/52

ChaosLeung commented 4 years ago

Thanks for your contribution, but I think this is unnecessary, because EditText's setInputType can achieve this.

alexcohn commented 4 years ago

setInputType introduces an unpleasant side effect, see how to force number keyboard when inputtype is set to numberPassword on Android. For EditText, there is a way to handle it with setTransformationMethod(), but this does not work for PinView.

ChaosLeung commented 4 years ago

For EditText, there is a way to handle it with setTransformationMethod(), but this does not work for PinView.

After doing some research, I found setTransformationMethod actually make the effect by using Spannable or replace characters with dots. The password dots in PinView is draw via Canvas, so setTransformationMethod make no effects in PinView.

setInputType introduces an unpleasant side effect, see how to force number keyboard when inputtype is set to numberPassword on Android.

You are right, adding a new method is the most appropriate approach.

alexcohn commented 4 years ago

Is it better now?

ChaosLeung commented 4 years ago

Great, I will release the new version as soon as possible, but it will take some time.

ChaosLeung commented 4 years ago

Hey @alexcohn , on second thought, I think isPasswordHidden should only works on textPassword, numberPassword and textWebPassword. Because for text, number and other "visible" types, the characters must always be visible unless the inputType is changed.

alexcohn commented 4 years ago

isPasswordHidden should only works on textPassword, numberPassword and textWebPassword

The trigger for this change was a request to keep the soft keyboard style unchanged when hiding and revealing the characters. The inputType dictates the keyboard style, e.g.

`number`

therefore it may be important for somebody to stay with the number style, as above, but hide the characters.

Anyways, because this addition does not force the user to do what they don't want, and does not break the legacy scenario, it's prudent not to impose unnecessary restrictions on it. Note that changing inputType resets the character visibility to default.

ChaosLeung commented 4 years ago

therefore it may be important for somebody to stay with the number style

I think if anyone really wants to use number, it means he doesn't want the password dots. If he needs to hide the numbers, he should use numberPassword.

Configure the inputType to "**Password" and use setPasswordHidden to hide or show the characters, so that we don't need to change the inputType and the soft keyboard layout will not be changed.

Another reason is that there are three methods (setInputType, setRawType, setKeyListener, API 28) in TextView that will modify the inputType. If isPasswordHidden is related to the inputType, we need to override and add code to the other two methods. In the long run, maybe not so great.

alexcohn commented 4 years ago

I understand why you worry about setRawType() and setKeyListener(). I am not sure these were reasonably supported by PinView before. My minimal experimentation with the vanilla EditText shows that the system widget (on Android 10) does not work well when these public methods are called at runtime (maybe it makes more sense to call them when the EditText widget is initialized).

More importantly, this change is only relevant for TYPE_CLASS_NUMBER, but I don't check that. This restriction could be imposed in different ways; for example, by using a specialized class of NumPinView which extends PinView.

Definitely, these limitations of setPasswordHidden() should be documented. For non-numeric case, the recommended technique should be toggling between TYPE_TEXT_VARIATION_VISIBLE_PASSWORD and `TYPE_TEXT_VARIATION_PASSWORD.

I believe that the developers can take responsibility for correct usage of the library; it's unlikely that a PinView will be a drop-in replacement for a regular EditText and inherit some calls like setRawType() that are not controlled by the developer.