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

[Screen Redesign] Light rearrangement of SeedAddressVerificationSuccess #651

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

Conversation

kdmukai
Copy link
Contributor

@kdmukai kdmukai commented Dec 30, 2024

Description

Fixes #650

Removes the sentence that had complex variable insertions in favor of simple single line reporting.

Before / After

Due to the slight UI changes, the master messages.pot translation strings have changed slightly and will need new translations.

Additional light refactors:

  • Clarify Exception message in screen.py
  • Skip LargeIconStatusScreen body text component if no text input param is provided. This allows this PR to simply reference self.components[-1] to reference the the last VISIBLE display element; in this case, the LargeIconStatusScreen.status_headline.
  • Light refactor to sort the imports in seed_screens.py.

This pull request is categorized as a:

  • Code refactor

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 1, 2025

Recording that in my review of this pr yesterday, that I'd found that the ui required two button-presses of "OK" instead of just one, and that I reported this directly to Keith for confirmation that he saw it too. At first, it felt like a "buggy" hardware button at my end.

Since this morning, confirmed by the fact that I couldn't find anything in this pr that explicitely causes this bug, Keith has worked on changes to hardware buttons that simplifies and corrects this error for me... So awaiting a pr that might precede this one (even though they're separate issues). Beyond that, this new screen and pr works for me, it will have my tested ACK as soon as the underlying buttons issue is resolved.

I am able to provoke the same strange behavior outside of this pr, in 'dev' at the 0.8.5-rc1 commit it happens as well. Picking a higher address index than a lower one will provoke this (not the 3rd address, pick the 93rd address to scan instead). Also, clicking quickly instead of waiting helps to avoid this "press OK again" behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 👀 Needs Code Review
Development

Successfully merging this pull request may close these issues.

SeedAddressVerificationSuccess: Refactor screen to be more l10n-friendly
2 participants