selfcustody / krux

Open-source signing device firmware for Bitcoin
https://selfcustody.github.io/krux/
Other
185 stars 37 forks source link

Adds "< " to "Back" menu items #420

Closed jdlcdl closed 4 months ago

jdlcdl commented 4 months ago

Description

Perhaps with another "arrow" glyph, this will look nicer, but "< Back" is still very identifiable as a special menu item.

Also changed some tuples' 2nd item to the more standard "lambda: MENU_EXIT" instead of "lambda: None"

What is the purpose of this pull request?

codecov[bot] commented 4 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Please upload report for BASE (develop@9eb1bcb). Learn more about missing BASE report. Report is 14 commits behind head on develop.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## develop #420 +/- ## ========================================== Coverage ? 94.63% ========================================== Files ? 57 Lines ? 7078 Branches ? 0 ========================================== Hits ? 6698 Misses ? 380 Partials ? 0 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

odudex commented 4 months ago

The lambda: MENU_EXIT didn't change behaviors, right?

tadeubas commented 4 months ago

Hi @jdlcdl ! Can we create a constant for Back and reuse it everywhere?

jdlcdl commented 4 months ago

The lambda: MENU_EXIT didn't change behaviors, right?

I was trying to standardize exiting menus with this MENU_EXIT status, but often it is ignored and doesn't matter. I wasn't intending to alter behavior. I'll move these back to the way they were because I'm not sure if they're inspected elsewhere.

odudex commented 4 months ago

The lambda: MENU_EXIT didn't change behaviors, right?

I was trying to standardize exiting menus with this MENU_EXIT status, but often it is ignored and doesn't matter. I wasn't intending to alter behavior. I'll move these back to the way they were because I'm not sure if they're inspected elsewhere.

I was just asking because I didn't know too, but if you tested it's ok!

jdlcdl commented 4 months ago

Hi @jdlcdl ! Can we create a constant for Back and reuse it everywhere?

Still not ready, but working in that direction.

latest commit tries to implement a cta_back() reusable in pages/init.py but to pass tests, couldn't use it in one place, and to pass i18n checks, had to feed it i18n food at least once. Still a work in progress, and have not tested it on device (only limited testing in simulator so far). It's leading me to believe that "Menu" class might take an optional cta_back param and set this up by default so that every menu doesn't always need it; then it would be easier to move from Menu item to Page later.

jdlcdl commented 4 months ago

Altering to ready-for-review.

odudex commented 4 months ago

Reviewed, tested and merged. Thank you!