Draw every "choose among N" screen as one list - #160
Merged
aido merged 2 commits intoAug 12, 2026
Merged
Conversation
Four screens ask the user to pick one of a fixed set of entries -- the menu of intentions, the BIP-39 phrase length, the PIN length, and the list of BIP-85 secrets. One of them was the SDK's list; the other three were stacks of nbgl_button_t built by hand. They are all the list now, through display_choice_list(), and the rule is written above it rather than left to be rediscovered on the next screen added. The stack grows upwards from the bottom margin, so the entry that pushes it too far is drawn over the title, and no test can see it: Speculos reports every text event at its full height with its full text. That is what the secret list hit when it gained a fourth entry. A list paginates instead. Two more things go with the component. The entries read top-down in the order they are written, where the stack read bottom-up. And there is one back arrow, the SDK's, where generic_screen_set_back_button() drew a second of its own geometry that coincided with it on today's three devices by arithmetic rather than by design. layout_generic_screen.c has no callers left and goes with them. The two length screens lose their icon and their black entry. The icons this repository authored name formats and these screens ask a quantity; a black control in this application means an act with a consequence, and "the largest of three amounts" was a third meaning for that signal -- on "How long is your Recovery Phrase?" it answered a question about a fact with a recommendation. The PIN screen loses a real one by it, and the comment where the emphasis used to be says so. Three titles lose the "\n" they carried: the list header wraps on words by itself, where the hand-built title areas left `wrapping` clear and broke on characters. Measured before concluding, on captures of both forms on all three devices: a list of three entries does leave the bottom of the screen empty, about 45% of Stax and 35% of apex_p. That is the cost, and it is written down with what it buys.
Three of the four "choose among N" screens changed idiom, so their tests change driver. genericlayout.py carried a table of touch coordinates counted from the bottom of the screen, because the hand-built stack grew upwards from the bottom margin and its first entry was the last line drawn. Nothing counts backwards any more: ragger's ChoiceList already knows where the rows of an SDK list are, so choicelist.py replaces the table with names alone -- the four intentions, the four secrets, the three phrase lengths and the three PIN lengths. The two length screens keep the numbers they had, which is a coincidence worth naming rather than relying on: the stack was written 12, 18, 24 and drawn bottom-up, so the shortest was already row 1 from the bottom and is row 1 from the top now. The callers say WORDS_12 and DIGITS_6 instead, so nobody has to know that. test_menu_positions.py still holds the mapping the only way that settles it, by touching each constant and asserting the screen it arrives on. The back arrow of the PIN length screen is the SDK's header arrow now, so its test drives UseCaseSubSettings().exit() -- and the comment warning that the application's own square could stop overlapping the SDK's header on a future device goes with the square.
Author
|
Every screen of the four journeys, on the three touch devices, on this Check — Straight from the menu to the length: entering the Phrase *is* the task, so this walk has no explanation in front of it. (5 screens)
Backup — The long one, and the one that crosses the most families: two explanations, two keypads, and a review whose last page is the warning. (11 screens)
Recover — Short as far as the keyboard; what follows is twenty-nine ByteWords per share. (4 screens)
Derive — The only walk that crosses **two** screens of the family in a row -- the secret list, then the PIN length. (11 screens)
|
Owner
|
Hi @buzzromain, A lot of these recent changes will make it easier to migrate the Nano X and Nano S+ to nbgl in the future. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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 touch stack carried two idioms for the same question. Four screens ask the
user to pick one of a fixed set of entries -- the menu of intentions, the
BIP-39 phrase length, the PIN length, the list of BIP-85 secrets -- and one of
them was the SDK's list while the other three were hand-built stacks of
nbgl_button_t. This gives all four the same component and writes down therule, at the point where the family is built, rather than leaving it to be
rediscovered on the next screen added.
Why the list and not the stack
The stack grows upwards from the bottom margin, so the entry that pushes it
too far is drawn over the title. That is not hypothetical: it is what
happened when the secret list gained a fourth entry, and Flex drew "PIN"
across the second line of the question. Nothing in a test could see it --
Speculos reports every text event at its full height with its full text, which
is the silence
reviews.assert_body_clears_button()exists to break. A listpaginates instead.
Two more things go with it: the entries read top-down in the order they are
written (the stack read bottom-up, so the tests counted from the end the code
did not), and there is one back arrow instead of two geometries --
generic_screen_set_back_button()drew aBUTTON_DIAMETERsquare 4px fromthe top, against the SDK's
BACK_BUTTON_HEADER_HEIGHTband, and the twocoincided on the three current devices by arithmetic rather than by design.
src/nbgl/layout_generic_screen.chad no callers left and is gone with them.Measured, not assumed
The objection to the list was that three short entries on a large screen would
look empty. Both forms were captured on all three devices before this was
settled: the list does leave the bottom of the screen empty -- about 45% of
Stax, 35% of apex_p -- and that is the cost. It buys the reading order, the
single back arrow, and the removal of a class of defect nothing could check.
A short list at the top of a large screen is also what the SDK's own settings
screens look like.
The subtitle a bar can carry does not reach this component:
nbgl_layoutBar_thas asubTextfield, but thenbgl_contentBarsList_tthat a generic configuration takes is texts and tokens and nothing else.
What else changed on those screens
BIP39_ICONandBIP85_ICONwere on the two length screens.The icons this repository authored name formats, and these screens ask a
quantity.
black. A black control in this application means an act with a consequence;
"the largest of three amounts" was a third meaning for that signal, and on
"How long is your Recovery Phrase?" it answered a question about a fact
with a recommendation. The PIN screen loses a real recommendation by it, and
the code says so where the entry used to be emphasised.
\n. The list header wraps on words by itself. The threetitles that carried a break carried it because the hand-built title areas
left
wrappingclear and broke on characters.them downwards because it was written upwards.
Rules written down
One per family, in the file where the family is built:
display_choice_list();owns a title, at the top of
src/nbgl/ui.c-- these three are signals usedacross families, so no single screen could settle them.
Deliberate divergences keep their reason at the site of the divergence: the
menu's absent icon, the Check journey's absent explanation, the grey
tap-to-continue against the black button on explanations, the icons on
verdicts and warnings, and the warning as the review's last page.
Tests
The three screens that changed idiom changed driver:
genericlayout.py, whichcarried a table of coordinates counted from the bottom, is replaced by
choicelist.py, which carries names only -- ragger'sChoiceListalreadyknows where the rows are.
test_menu_positions.pystill holds the mapping thesame way, by touching each constant and asserting the screen it arrives on.
Verification
invocation: nanox 13 passed, nanosp 13, stax 32, flex 32, apex_p 32, and
no failure on any of them;
grep -rnE '%\.\*s|%s' src/clean,clang-format --dry-run --Werrorcleanover all of
src/;Flash and RAM
RAM is
_ebss - _bss; thebsscolumn ofsizeis a fixed region here.The three Nano targets are byte-identical: nothing in this touches the
two-button stack.
Captures
On the orphan branch
screenshots/one-idiom-per-choiceof the fork, as theother capture branches are. Each screen on stax, flex and apex_p, at half the
device's real width so the three stay comparable to each other.
The menu of intentions
Four entries. The header wraps the question on words by itself, where the hand-built title area broke on characters and carried a placed
\n.The BIP-39 phrase length
The icon named a format over a question about a quantity; the black entry answered a question about a fact with a recommendation. The lengths now read upwards, 12 / 18 / 24, where the stack read them down because it was built up.
The BIP-85 secret list
Unchanged -- already this component. It is here because it is what the other three now look like, and because its fourth entry is what ended the stack.
The PIN length
Same two removals as the phrase length. This is the screen that loses a real recommendation by it; see the note at the end.
One thing left open for review
The PIN length screen loses "8 digits" drawn black, which said "the safest of
the three". A bar carries one line of text and no emphasis, and the one
meaning left for a black control is an act with a consequence -- so the
recommendation is not said anywhere now. Restoring it would mean a
CHOICES_LIST with an initChoice, or putting it in the label; neither is in
this change, and the comment where the emphasis used to be says the loss is
paid rather than overlooked.