search

owncloud / android

:phone: The ownCloud Android App
GNU General Public License v2.0
3.77k stars 3.08k forks source link

[FEATURE REQUEST] Password generator for public links #4349

Closed JuancaG05 closed 2 months ago

JuancaG05 commented 3 months ago

Related Issues

App: https://github.com/owncloud/android/issues/4308

QA

Test plan: https://github.com/owncloud/QA/blob/master/Mobile/Android/Executions/Release_4.3/Password%20Policy%20-%20Generator.md

Reports:

jesmrec commented 3 months ago

(1) [FIXED]

We could have a problem if the translation of Generate password is too long in some languages. I guess, there will not be enough room for the copy option or it will be misplaced somehow.

Since the focus is in the Password field, should it be enough with the label Generate? (iOS approach) wdyt? @JuancaG05 @Aitorbp

jesmrec commented 3 months ago

(2) [FIXED]

oC10 servers are showing the options generate password and copy even though they don't support the feature. It's not a bad idea to let users generate secure passwords, also if they are not mandatory or must not fulfill any policy.

In that case, options should be hidden and only displayed if the user enables the password switch. Also the behaviour of copy button differs (no policy that leads its enable/disable status)

Pixel 2, Android 11 9acf1fe75

JuancaG05 commented 3 months ago

Since the focus is in the Password field, should it be enough with the label Generate? (iOS approach) wdyt? @JuancaG05 @Aitorbp

Ok! Yes, I think it's enough. Will change it 👍

JuancaG05 commented 3 months ago

About (2), in fact, the behaviour of "Generate password" button would also differ, since the password is generated in function of the policies coming in the capabilities, and not sure if that would be working if the passwordPolicy (not only the different policies) itself is null. I would say it's better to hide it for oC10 servers to avoid complexity about this, what do you think? @jesmrec

jesmrec commented 3 months ago

About (2), in fact, the behaviour of "Generate password" button would also differ, since the password is generated in function of the policies coming in the capabilities, and not sure if that would be working if the passwordPolicy (not only the different policies) itself is null. I would say it's better to hide it for oC10 servers to avoid complexity about this, what do you think? @jesmrec

IMO, "password policies" and "generate passwords" should be (theoretically) isolated features. Not sure if we can set any "default" policies in case there is no server ones, but, this is out of the scope here. For me in this PR are valid any of these two:

Pick the easiest one for you, then, we will decide whether there is place for improvement in another moment.

JuancaG05 commented 3 months ago

IMO, "password policies" and "generate passwords" should be (theoretically) isolated features. Not sure if we can set any "default" policies in case there is no server ones, but, this is out of the scope here. For me in this PR are valid any of these two:

  • Remove options from oC10 servers, so that, the feature is only available in oCIS
  • Let users generate passwords with any default policy (good to have but not mandatory here)

Pick the easiest one for you, then, we will decide whether there is place for improvement in another moment.

We can set default policies, and indeed they are already set, but as I commented, it depends on the structure of the capabilities, which in terms of password policy is not the same in oC10 and oCIS. To avoid more technical complexity here, I'll just remove the options from oC10 servers and we can think about this in a future 👍

jesmrec commented 3 months ago

(1) and (2) fixed

jesmrec commented 3 months ago

(3)

I noticed a tiny detail: when clicking on Generate, the default is that the password is initially visible. But, if user types it, it is initially hidden. I think that both should behave in the same way, i'd say with a default hidden password.

Pixel 2, Android 11 d17c425b

JuancaG05 commented 3 months ago

(3)

I noticed a tiny detail: when clicking on Generate, the default is that the password is initially visible. But, if user types it, it is initially hidden. I think that both should behave in the same way, i'd say with a default hidden password.

The default as seen in other apps is that the text is hidden in this kind of text fields. However, I decided to do it like this because that text is not typed by the user, so that they can see which password was generated, and then if they want, hid it again. I see it as a different use case, and I think I've seen it this way on other apps, but now I can't think of any example 😵‍💫. Maybe @tbsbdr can enlighten us 😃

tbsbdr commented 3 months ago

I'd follow the following:

Whenever users interact with a system, they need to know whether the interaction was successful.

Source: Visibility of System Status (Usability Heuristic #1)

I hope that makes sense. thanks for paying attention to these details!

jesmrec commented 3 months ago

Then, current approach will stay as is

jesmrec commented 3 months ago

This is approved on my side