Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add SPACE, DEL icons to Keyboard; various UI consistency tweaks #664

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

kdmukai
Copy link
Contributor

@kdmukai kdmukai commented Jan 19, 2025

Description

See affected screenshot changes: SeedSigner/seedsigner-screenshots#5

Main purpose:

UI fixes:

  • We had never used the ERROR icon (we were using the WARNING exclamation in red instead).

UI consistency tweaks:

  • Slightly opinionated edits for when to use ERROR vs WARNING and red vs yellow.
  • General principles applied:
    • Red = Extreme danger (displaying mnemonic, PSBT addr verification failed, etc)
    • Red = System error, bug, showstopper
    • Yellow = Warning; be aware but don't scare the user.
    • Yellow = Incompatibility (e.g. unknown QR format). Yes, something went wrong, but don't scare the user.
  • Judgment call: Signing PSBT didn't produce a valid signature: I used WARNING in yellow.
    • Rationale: maybe they already signed the multisig PSBT with this key. That's not a reason to panic. Or they opted to try to sign with a key that we didn't think could sign in the first place (has the "(?)" on the selection screen).

Minor misc:

  • Keyboard: Use our CHEVRON_LEFT / CHEVRON_RIGHT for cursor navigation.
  • I/O Test Screen: Use our icons instead of FontAwesome.
  • Remove legacy load_icon() function which was an early ~v0.5.0 relic.
  • Minor spacing fix at the bottom edge of ButtonListScreen where is_bottom_list=True with more than one button.
  • Render Dice icons to fill the Keyboard key size.
  • Change to text when multisig self-transfer addr can't be verified by descriptor; edited to fit better in available space. Text change affects translations (messages.pot).
  • Applied "Suspicious PSBT" consistently across single sig and multisig.
  • Screenshot generator tweaked to generate same results as the ScanView error when it scans an unsupported QR format.

This pull request is categorized as a:

  • UI cleanup

Checklist

  • I’ve run pytest and made sure all unit tests pass before sumbitting the PR

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

  • N/A

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

@jdlcdl
Copy link

jdlcdl commented Jan 19, 2025

As of 5c6c99b

This is working for me. Have reviewed code changes, all reasonable to me.
Have tested a little, but will test more, on pi0 built by seedsigner-os pr-83
Have carefully reviewed screenshots in english, compared to merges made mid-last week, all as expected given the code changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🏗 In Progress
Development

Successfully merging this pull request may close these issues.

2 participants