xmr: UI QR code fix, zoom by swipe #1074
Conversation
ph4r05
force-pushed
the
pr/004-xmr-qr-scrollzoom
branch
from
1e3a621
to
efe15fd
Compare
| def render_scrollbar(pages: int, page: int) -> None: | ||
| BBOX = const(220) | ||
| SIZE = const(8) | ||
|
|
||
| padding = 14 | ||
| if pages * padding > BBOX: | ||
| padding = BBOX // pages | ||
|
|
||
| X = const(232) | ||
| Y = (BBOX // 2) - (pages // 2) * padding | ||
|
|
||
| for i in range(0, pages): | ||
| if i == page: | ||
| fg = ui.FG | ||
| else: | ||
| fg = ui.GREY | ||
| ui.display.bar_radius(X, Y + i * padding, SIZE, SIZE, fg, ui.BG, 4) |
There was a problem hiding this comment.
Only X was changed in this code compared to the render_scrollbar from trezor/ui/scroll.py.
Question: Do you prefer to
- a) extend the original function so X can be passed as a param (I assume no as it complicates API for just this use-case) or
- b) copy the code locally with minor modification?
|
ph4r05
force-pushed
the
pr/004-xmr-qr-scrollzoom
branch
from
efe15fd
to
ee7a4e7
Compare
|
ad 1 - I've added a change that shows buttons only for the small QR code version. However, such changes to the underlying Confirmation dialog may be harder to maintain so I would not be surprised if we just choose to keep the buttons there w.r.t. change costs. ad 2 - I like the variant with all gestures toggling the QR code sizes as there is IMO a higher chance user will get the control logic right faster. Any swipe toggles the state. As there are just 2 I don't see a reason why it should be more strict. |
|
@tsusanka Can you please discuss this with the product how do you want his to be implemented? |
|
current master f382f77 changed overlay of buttons
|
I think the fix proposed by ph4r05 fixes it. |
|
@ph4r05 can't we simply keep the down-scaled QR image and that's it? As in:
without any pagination whatsoever. It seems to be readable just fine by my phone. |
|
@tsusanka if it is readable fine in |
This is a draft PR addressing monero-project/monero-gui#2960 (reported by @bosomt) by:
Current state:
Screen where user can activate the QR code screen:
Initial QR screen state
Zoomed QR code after swiping to any direction (scale = 5)
After another swipe the scale returns back to 3 (swipe toggles between 3 and 5)
This is just an idea of how to address this issue, it can be probably implemented in a more elegant way.
Edit: I've added a new scroll-bar to give a user a hint this view is scrollable. I had to offset to the right so it does not collide with the QR code.