Skip to content

Conversation

kdmukai
Copy link
Contributor

@kdmukai kdmukai commented May 25, 2025

The problem / background

The TextArea.allow_text_overflow param will intentionally raise an exception if set to False when the provided text does not fit within the TextArea dimensions.

The original idea was to immediately bring this to a developer's attention when building a new Screen.

But now in the era of l10n support, we have to assume that any text could inadvertently overflow, no matter how strict we think a particular part of a screen might need to be.

For example, the current Norweigian translations are erroring out of the screenshot generator for this exact reason:

Running SettingsEntryUpdateSelectionView_coordinators

...

  File "/Users/kdmukai/dev/seedsigner/tests/screenshot_generator/generator.py", line 416, in screencap_view
    screenshot_config.View_cls(**screenshot_config.view_kwargs).run()
  File "/Users/kdmukai/dev/seedsigner/src/seedsigner/views/settings_views.py", line 182, in run
    ret_value = self.run_screen(
  File "/Users/kdmukai/dev/seedsigner/src/seedsigner/views/view.py", line 112, in run_screen
    self.screen = Screen_cls(**kwargs)
  File "<string>", line 23, in __init__
  File "/Users/kdmukai/dev/seedsigner/src/seedsigner/gui/screens/settings_screens.py", line 35, in __post_init__
    self.components.append(TextArea(
  File "<string>", line 26, in __init__
  File "/Users/kdmukai/dev/seedsigner/src/seedsigner/gui/components.py", line 457, in __post_init__
    self.text_lines = reflow_text_for_width(
  File "/Users/kdmukai/dev/seedsigner/src/seedsigner/gui/components.py", line 1909, in reflow_text_for_width
    raise TextDoesNotFitException("Text cannot fit in target rect with this font+size")
seedsigner.gui.components.TextDoesNotFitException: Text cannot fit in target rect with this font+size

Solution / rationale

Removing allow_text_overflow allows the screenshot to always be rendered, even if the resulting text layout is bad. It is then up to the developer or, more likely, the translator to adjust the text to fit (or open an Issue to say that more room should be created for that text).

Removing it also gives us the freedom to launch a translation as-is, even if the text doesn't quite fit but the spacing is deemed to be acceptable enough.

Most importantly, if this param remained AND if such an overflow exception happened on one of the few screens that doesn't have a screenshot (and if hands-on testing misses it as well), we wouldn't know beforehand that the screen will fail. However unlikely that all might be, end users would experience the exception in the UX, resulting in a showstopper bug for them for that language.

Testing / side-effects

Removing the allow_text_overflow attr is equivalent to changing all prior references to True (allow overflow in all cases). In the few cases where it was originally False, the exception noted above would be raised. So for any screen in a given language that did NOT raise that exception, we'd expect that it would still work completely fine after this change (i.e. no effect).

I re-ran the screenshot generator against the 14 languages that are the furthest along. I can't say that I inspected every last detail but a cursory spot check didn't reveal any unexpected side effects. And all 14 could now successfully complete (prior to this change, 13 would complete while Norweigian always failed).

If you'd like to play with all these translations, see my max_locales branch: https://github.com/kdmukai/seedsigner-translations/tree/max_locales


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:

  • Raspberry Pi OS Manual Build
  • SeedSigner OS on a Pi0/Pi0W board
  • Other: running the test suite and screenshot generator in local dev

@alvroble
Copy link
Contributor

Tested with your max_locales and ACK. I also did an overall review and all 14 locales seems to be okay. The only screenshot that seems to still not be generating correctly for languages with non-latin alphabets is PSBTOpReturnView_raw_hex_data (SeedSigner/seedsigner-translations#30 (comment)), which I think is not critical at all.

@kdmukai
Copy link
Contributor Author

kdmukai commented Jun 28, 2025

Screenshot generation for the initial Thai translations (SeedSigner/seedsigner-translations#43) is blocked until this PR is merged.

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

Successfully merging this pull request may close these issues.

3 participants