SeedSigner / seedsigner

Use an air-gapped Raspberry Pi Zero to sign for Bitcoin transactions! (and do other cool stuff)
MIT License
727 stars 170 forks source link

[Enhancement] MicroSD toast timer settings #600

Open alvroble opened 2 months ago

alvroble commented 2 months ago

STILL IN PROGRESS

Description

Proposed solution for issue #546

Provides 3 microSD options:

SettingsEntryUpdateSelectionView_microsd_toast_timer

For the third option, the new warning dialog UI (re-used and old View class that was unused) looks as follows:

RemoveMicroSDWarningView

So the user can choose between:

This behaviour has the possibility to be different. In previous commit https://github.com/SeedSigner/seedsigner/commit/255464783009dd5caae1077d2486acc3e8267c14, a fullscreen toast with no dialog options is shown instead of the warning dialog. Here, user must remove MicroSD to continue. However, as persistent settings must be written to the MicroSD to continue, user should have to insert it again if they wanted to hange the settings.

That behaviour was a bit more confusing for the user but is still an option if most devs agree on it.

This pull request is categorized as a:

Checklist

If you modified or added functionality/workflow, did you add new unit tests?

I have tested this PR on the following platforms/os:

newtonick commented 2 months ago

Comment related to this PR left here: https://github.com/SeedSigner/seedsigner/issues/546#issuecomment-2332525097

I live the setting options, but option 3 still needs work.

alvroble commented 2 months ago

Thanks @newtonick for your comments. Just to continue here the discussion. I'll mark this PR as draft until we clarify the third option UX and flow.

I will test how can we manage to have this warning screen:

363503351-a21c6291-6de3-4137-9843-0636a42ddea0

With this behavior:

In the settings, as @newtonick proposed, we could make it work as if

when the user selects the back "<" arrow, the "Remove Memory Card" would re-evaluate the state of things and proceed accordingly.

I think this may be too complex of a refactor but a nice feature to have.