search

odk-x / tool-suite-X

ODK-X Tool Suite Project roadmaps, issue queue, release notes and wiki.
https://www.odk-x.org
Apache License 2.0
25 stars 43 forks source link

ODK-X Services Server Settings Server Password Visibility Icon Is Incorrect #257

Open clarlars opened 3 years ago

clarlars commented 3 years ago

When a user enters the Server Password in Server Settings, the visibility icon shows the password when visibility is hidden and hides the password otherwise (see attached image). It should be the other way around. This happens in ODK-X Services 2.1.8 rev 242p.
password_visibility_icon

sarahelshabrawy commented 3 years ago

I can work on it

wbrunette commented 3 years ago

@sarahelshabrawy I have assigned the issue to you

pawanharariya commented 3 years ago

@clarlars I searched through login of few sites and found out that this has two different interpretations: one that you suggested and second one where the icon implies the state you want to switch to and not the current state.

For example: sites like grammerly, mailchimp, instagram,etc use this implementation. Screenshot (81) Screenshot (82)

However facebook uses the implementation that you suggested. Screenshot (83)

If we want to go by the same implementation then there are two other screens as well that require the change:

  1. Admin password creation screen
  2. Admin password challenge screen
    • server password screen ( already mentioned above)
sarahelshabrawy commented 3 years ago

@pawanharariya I thought of the same thing as well. It's somehow confusing to be honest, however, I think that the implementation @clarlars suggested sounds more logical although I'm okay with both.

linl33 commented 3 years ago

I think we should use the default Google provided. I've seen seen this implemented both ways. But the default that comes with the libraries is fine. So if the visibility icon was modified, IMO, we should restore to the default but if this is the stock settings I think it's fine.

clarlars commented 3 years ago

This issue stood out to me as it deviates from the previous behavior, but in the end, I am also fine with the default behavior provided by Google if no one has any objections.

sarahelshabrawy commented 3 years ago

That seems sensible, I'm fine with it as well, Just let me know if we're going to close this issue before I start working on it.

wbrunette commented 3 years ago

@sarahelshabrawy what is the google default? Please verify that we are following the Android standards. Once you do that we can close this.

sarahelshabrawy commented 3 years ago

Yes, I understood. Just wanted to make sure that everyone is okay with it.. I will go ahead then

linl33 commented 3 years ago

The example shown on Google's Material Design website (https://material.io/components/text-fields/android#using-text-fields) implements the password visibility toggle as implemented by @pawanharariya.

@sarahelshabrawy I see that you've opened a pull request to invert the icons, did you find a source that recommends differently?

pawanharariya commented 3 years ago

@linl33 Yes the current implementation is the default one. However it was implemented by @VarunT11 and not me. I just found this issue interesting and was sharing my views.

sarahelshabrawy commented 3 years ago

@linl33 As @pawanharariya mentioned in his comment the default behaviour provided by material design is not implemented by all sites, like facebook .. Both implementations are okay, even material design used to implement the suggested implementation in previous versions. But as mentioned it was agreed to apply Android standards. Just let me know please if I misunderstood anything.