[Refactor] [l10n] Remove TextArea.allow_text_overflow
for improved localization / screenshot usability
#761
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The problem / background
The
TextArea.allow_text_overflow
param will intentionally raise an exception if set toFalse
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:
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 toTrue
(allow overflow in all cases). In the few cases where it was originallyFalse
, 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_localesThis pull request is categorized as a:
Checklist
pytest
and made sure all unit tests pass before sumbitting the PRIf you modified or added functionality/workflow, did you add new unit tests?
I have tested this PR on the following platforms/os: