UI: Context arrows, customizable colors, and a page index indicator for menu items
What is this PR for?
- Add context arrows to menu items
- Add custom color to menu items
- New pages index on menus with more than one page
- Add
< Backon missing menus - Improved
draw_keyset_indexon the keypads to make it more visible - Double outline on keypads when using buttons to navigate on touch devices
- New Swipe Threshold setting
- Increased value range for settings Touch Threshold and Buttons Debounce
- Reduced setting Buttons Debounce default value (customized default for M5StickV)
Changes made to:
- [x] Code
- [x] Tests
- [ ] Docs
- [x] CHANGELOG
Did you build the code and tested on device?
- [x] Yes, build and tested on Amigo, Embed Fire and M5StickV
What is the purpose of this pull request?
- [ ] Bug fix
- [x] New feature
- [ ] Docs update
- [ ] Other
Codecov Report
:x: Patch coverage is 99.75124% with 1 line in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 97.42%. Comparing base (3ddefc5) to head (bc9c7ab).
:warning: Report is 4 commits behind head on develop.
| Files with missing lines | Patch % | Lines |
|---|---|---|
| src/krux/touch.py | 97.95% | 1 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## develop #771 +/- ##
===========================================
+ Coverage 97.37% 97.42% +0.04%
===========================================
Files 83 83
Lines 10534 10673 +139
===========================================
+ Hits 10258 10398 +140
+ Misses 276 275 -1
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
I've removed the swipe_threshold customization from this PR in favor of using an industry-standard approach. The required distance depends on the screen’s PPI: higher-PPI 2-inch displays (Yahboom, Wonder, TZT) need more physical pixels than the lower-PPI 3.5-inch Amigo to feel equally natural.
Industry guidelines use roughly 20–30 px for swipe detection on 160–200 PPI screens (and about half that for smartwatches). Based on this, I lowered our threshold from 50 px to 35 px, which is still a conservative value.
Fixed a swipe-handling issue: devices now reject swipes made at diagonal angles (an industry-standard behavior). A diagonal swipe is only accepted if the angle is below 27°, meaning one axis must have at least twice the movement of the other.
Industry standards say a swipe shouldn't exceed ~1 s (average user completes a swipe in ~450 ms). Our implementation allowed swipes even after a 10-second hold. I’ve now capped swipe duration at ~750 ms.
ACKt, test on amigo, and it works as expected. Arrows make navigating throughout the wallet much more intuitive, and the touch-feedback highlighting is an excellent visual improvement as well when navigating. Nice work!
Only small thing I would suggest is that when the theme is orange, for example the red shutdown lettering would look better if it were yellow I think.
Just wrapped up the test coverage for this PR, it came out to about ~450 new lines (~200 of those are tests).
Here’s how I think it’s best to review it:
-
First, check that no existing tests were removed or altered in a way that hides a real issue. In other words, confirm that nothing was changed just because a code update broke a test
-
Second, flash a touch-enabled device and verify the behavior described in the PR’s “What is this PR for?” section. You can compare it against another device running
developor the latest release to spot differences (if you want / can do this with a non-touch device too) -
Third, note that the code changes are all within input/touch, pages and other UI-related parts. Because of that, the hands-on device testing you did in step 2 is far more valuable than doing a deep code review. So no need to spend too much time trying to dissect every line of the diff
Built and tested on the wonder_k. Initial impression is very positive.
Note: the < | Go | > style pages do not have the touch highlight.
For screens with a < Back option, maybe worth considering supporting swipe (from left to right) as well?
I'm lukewarm on the menu context arrows. I find the button labels themselves sufficient to let me know where I want to go next.
The context arrows are fairly intuitive within Settings but since the Krux UI is almost always list-based regardless, the presence or absence of the context arrow doesn't provide much info to prime the user's intuition, imo.
And I think they're almost slightly confusing or inconsistent in other areas. For example: within "New Mnemonic"; to me "Via Camera" etc are clicks into the options for a given feature (so a different context), whereas the context arrow makes the most sense for nested lists within the same context (e.g. the "Encryption" settings within "Settings").
Also it does have some slightly negative effects on worst-case alignment:
In this case I'd argue that the ">" should be pinned to the right edge and vertically centered in the row.
Note: the
< | Go | >style pages do not have the touch highlight.
I didn’t update those category-settings screens because they’re very simple, and I figured the visual change would already be clear there. But if you think it feels inconsistent since the other screens have touch feedback, I can add it, no problem.
For screens with a
< Backoption, maybe worth considering supporting swipe (from left to right) as well?
I’m not sure… for now, swipe up/left goes to the next page, and swipe down/right goes to the previous one on menus that have multiple pages. Only a few menus actually do, like: Tools > Device tests > Check SD Card, Settings > Hardware > Printer > CNC, or when exporting QR codes as images or using Datum convert. On the M5StickV, a few more menus can span multiple pages.
In this case I'd argue that the ">" should be pinned to the right edge and vertically centered in the row.
I think the most valuable feedback will come from users once we release, and then we can refine it further.. It’s an interesting idea, but since we may not have enough space to print the arrow when the text uses all available pixels, we could end up with something like this:
But if you think it feels inconsistent since the other screens have touch feedback, I can add it, no problem.
Yeah, I think this sort of feedback becomes a subconscious expectation and then feels a tiny bit jarring when it's missing.
During SeedSigner's UX overhaul, Easy really held my feet to the fire to ensure we were being consistent everywhere. Tough pages required a tiny bit of extra flexibility in a handful of cases, but then even those exceptions were made to at least conform to each other (i.e. "you can't violate principle X, Y, or Z!" "Um, well, I need to violate Y here, here, and here." "Errg, fine, but only do it using approach "Y.b.").
since we may not have enough space to print the arrow when the text uses all available pixels, we could end up with something like this
Ya, probably more work than it's worth, but you'd need some sort of smarter draw_hline (or somewhere around there): If there's room inline, render the context arrow attached to the text. If it's forced to multiline, detach the context arrow and respect the space.
Yeah, I think this sort of feedback becomes a subconscious expectation and then feels a tiny bit jarring when it's missing.
Sure will do this!
If there's room inline, render the context arrow attached to the text. If it's forced to multiline, detach the context arrow and respect the space.
This I would leave to another PR :wink:
Hey @kdmukai it is done :fist_oncoming: ! Now Category settings also have touch feedback highlight.