freeotp / freeotp-android

Apache License 2.0
1.44k stars 303 forks source link

"random" mode of FAB changes FAB icon incorrectly #298

Open dkozel opened 1 year ago

dkozel commented 1 year ago

Version 2.0 (40)

It took a while to figure out this "random mode" is likely a development tool? It would be nice to have comments in the code making the purpose clear. On balance I'm generally against having hidden development modes in deployed code, but am grateful for the overall app update.

Separately, the icon change is confusing and the logic seems backwards. Is it intended that the FAB icon change to make it clear that the "random" mode is active and then to reset to the normal icon after 15 seconds? Currently the behavior is the opposite.

https://github.com/freeotp/freeotp-android/blob/ab6ab2b565195ee919fbb552d58e31924b09a968/mobile/src/main/java/org/fedorahosted/freeotp/main/Activity.java#L591-L600

justin-stephenson commented 1 year ago

Version 2.0 (40)

It took a while to figure out this "random mode" is likely a development tool? It would be nice to have comments in the code making the purpose clear. On balance I'm generally against having hidden development modes in deployed code, but am grateful for the overall app update.

That's correct, though it is not used all that often. It was around prior to my working on the project.

Separately, the icon change is confusing and the logic seems backwards. Is it intended that the FAB icon change to make it clear that the "random" mode is active and then to reset to the normal icon after 15 seconds? Currently the behavior is the opposite.

https://github.com/freeotp/freeotp-android/blob/ab6ab2b565195ee919fbb552d58e31924b09a968/mobile/src/main/java/org/fedorahosted/freeotp/main/Activity.java#L591-L600

That's also correct, it should be fixed or we can simply remove this random mode entirely. I'm open to either.