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

solves issue #555 (Update keyboard.py) #598

Closed
wants to merge 5 commits into from

Conversation

fedebuyito
Copy link
Contributor

@fedebuyito fedebuyito commented Aug 29, 2024

Growning "del" button size property (from 2 to 3) solves navigation issue ("6th dice" -> "del").

solves issue #555

Description

Describe the change simply. Provide a reason for the change.

Include screenshots of any new or modified screens (or at least explain why they were omitted)

This pull request is categorized as a:

  • New feature
  • Bug fix
  • Code refactor
  • Documentation
  • Other

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?

  • No, I’m a fool
  • Yes
  • N/A

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

Note: Keep your changes limited in scope; if you uncover other issues or improvements along the way, ideally submit those as a separate PR. The more complicated the PR the harder to review, test, and merge.

Growning "del" button size property (from 2 to 3) solves navigation issue ("6th dice" -> "del").

solves issue SeedSigner#555
@newtonick
Copy link
Collaborator

This PR might fix the dice role entry screen but breaks other screens that use the delete keyboard. When testing the passphrase entry keyboard screen, it throws this exception: UnhandledExceptionView({'error': ['Exception', 'keyboard.py, 204, in __init__', ' charset will not fit in a 4x6 layout | additional_keys']}) | clear_history: True

This exception can also be seen when running the screenshot generator (pytest tests/screenshot_generator/generator.py)

FAILED tests/screenshot_generator/generator.py::test_generate_screenshots - Exception: charset will not fit in a 4x6 layout | additional_keys: [{'code': 'SPACE', 'letter': 'space', 'font': 'compact', 'size': 2}, {'code': 'CURSOR_LEFT', 'letter': '<', 'font': 'regular', 'size': 1}, {'...

@newtonick
Copy link
Collaborator

Other solution options are being discussed on #555

@newtonick newtonick added the failed test failed testing and needs correction prior to merge label Aug 30, 2024
Additional differents sizes "del" buttons
new custom_additional_keys prop on KeyboardScreen class for can to change additional button (KEY_BACKSPACE) on its childs classes
Changing KEY_BACKSPACE size on BIP39, SeedBIP85, SeedCustomDerivation, symbols_1, symbols_2 screens
Changing KEY_BACKSPACE size on ToolsDiceEntropy and ToolsCoinFlip screens
@fedebuyito
Copy link
Contributor Author

With further suggestions of @kdmukai I could to fix issues and failed test #598 . #555

@fedebuyito fedebuyito closed this Sep 6, 2024
@fedebuyito fedebuyito deleted the fedebuyito-patch-1 branch September 6, 2024 18:22
@fedebuyito fedebuyito restored the fedebuyito-patch-1 branch September 6, 2024 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
failed test failed testing and needs correction prior to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants