-
-
Notifications
You must be signed in to change notification settings - Fork 486
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add button to toggle password visibility #3894
base: master
Are you sure you want to change the base?
Conversation
android:id="@+id/password" | ||
style="@style/Input.Default" | ||
android:layout_width="0dp" | ||
<androidx.constraintlayout.widget.ConstraintLayout |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merge this constraint layout into the parent constraint layout. Traditional layouts are inefficient.
app:layout_constraintBottom_toBottomOf="@id/toggle_password_visibility_button" | ||
/> | ||
|
||
<ImageButton |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -514,6 +514,7 @@ | |||
<string name="past_week">Past week</string> | |||
<string name="scheduled_in_next_24_hours">Scheduled in next 24 hours</string> | |||
<string name="past_24_hours">Past 24 hours</string> | |||
<string name="show_or_hide_password">show or hide password</string> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<string name="show_or_hide_password">show or hide password</string> | |
<string name="show_or_hide_password">Show password</string> |
@@ -0,0 +1,5 @@ | |||
<vector xmlns:android="http://schemas.android.com/apk/res/android" android:height="24dp" android:tint="#000000" android:viewportHeight="24" android:viewportWidth="24" android:width="24dp"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please make sure both icons are formatted the same as all existing icons, including the same attributes for tint.
@@ -58,6 +60,10 @@ class UserLoginCredentialsFragment : Fragment() { | |||
} | |||
} | |||
|
|||
with (binding.togglePasswordVisibilityButton) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
with (binding.togglePasswordVisibilityButton) { | |
with(binding.togglePasswordVisibilityButton) { |
binding.togglePasswordVisibilityButton.setImageResource(R.drawable.ic_visibility_off) | ||
} | ||
|
||
binding.password.setSelection(binding.password.text.length) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this line does anything
Hey @noahvogt, are you still interested in finishing this pull request? it's currently waiting for you to address my review comments above. It's otherwise good to merge (if rebased). |
Add Button to Toggle Password Visibility
Changes
Add Button in
UserLoginCredentialsFragment.kt
to toggle the visibility of the password when logging in. This would be a big QOL improvement for me, as I am sure it is for others that don't have a proper keyboard connected to their TV's as well.Issues
Fixes #255
Preview
It looks like this: