Skip to content

Commit 03cae0d

Browse files
nroggeman-ledgernroggeman-ledger
authored
Merge pull request #1006 from LedgerHQ/support-more-than-3-pairs-in-address-review
Support more than 3 pairs in address review
2 parents 5565060 + 9ab1519 commit 03cae0d

File tree

5 files changed

+177
-103
lines changed

5 files changed

+177
-103
lines changed

‎lib_nbgl/include/nbgl_layout.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ extern "C" {
4949
#define MEDIUM_CENTERING_HEADER 56
5050
#define MEDIUM_CENTERING_FOOTER 88
5151
#define LONG_PRESS_BUTTON_HEIGHT 128
52+
#define UP_FOOTER_BUTTON_HEIGHT 120
5253

5354
#define PRE_TEXT_MARGIN 32
5455
#define TEXT_SUBTEXT_MARGIN 16
@@ -74,6 +75,7 @@ extern "C" {
7475
#define MEDIUM_CENTERING_HEADER 64
7576
#define MEDIUM_CENTERING_FOOTER 64
7677
#define LONG_PRESS_BUTTON_HEIGHT 152
78+
#define UP_FOOTER_BUTTON_HEIGHT 136
7779

7880
#define PRE_TEXT_MARGIN 28
7981
#define TEXT_SUBTEXT_MARGIN 14

‎lib_nbgl/src/nbgl_layout.c

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,6 @@
4545
#define FOOTER_HEIGHT 80
4646
#define BAR_INTERVALE 12
4747
#define FOOTER_BUTTON_HEIGHT 128
48-
#define UP_FOOTER_BUTTON_HEIGHT 120
4948
#define ROUNDED_AND_FOOTER_FOOTER_HEIGHT 192
5049
#define ACTION_AND_FOOTER_FOOTER_HEIGHT 216
5150
#define FOOTER_TEXT_AND_NAV_WIDTH 160
@@ -68,7 +67,6 @@
6867
#define FOOTER_HEIGHT 80
6968
#define BAR_INTERVALE 16
7069
#define FOOTER_BUTTON_HEIGHT 136
71-
#define UP_FOOTER_BUTTON_HEIGHT 136
7270
#define ROUNDED_AND_FOOTER_FOOTER_HEIGHT 208
7371
#define ACTION_AND_FOOTER_FOOTER_HEIGHT 232
7472
#define FOOTER_TEXT_AND_NAV_WIDTH 192

‎lib_nbgl/src/nbgl_use_case.c

Lines changed: 154 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -423,6 +423,8 @@ static uint8_t getContentNbElement(const nbgl_content_t *content)
423423
switch (content->type) {
424424
case TAG_VALUE_LIST:
425425
return content->content.tagValueList.nbPairs;
426+
case TAG_VALUE_CONFIRM:
427+
return content->content.tagValueConfirm.tagValueList.nbPairs;
426428
case SWITCHES_LIST:
427429
return content->content.switchesList.nbSwitches;
428430
case INFOS_LIST:
@@ -1066,9 +1068,26 @@ static bool genericContextPreparePageContent(const nbgl_content_t *p_content,
10661068
break;
10671069
}
10681070
case TAG_VALUE_CONFIRM:
1069-
memcpy(&pageContent->tagValueConfirm,
1070-
&p_content->content.tagValueConfirm,
1071-
sizeof(pageContent->tagValueConfirm));
1071+
// only display a TAG_VALUE_CONFIRM if we are at the last page
1072+
if ((nextElementIdx + nbElementsInPage)
1073+
== p_content->content.tagValueConfirm.tagValueList.nbPairs) {
1074+
memcpy(&pageContent->tagValueConfirm,
1075+
&p_content->content.tagValueConfirm,
1076+
sizeof(pageContent->tagValueConfirm));
1077+
nbgl_contentTagValueList_t *p_tagValueList
1078+
= &pageContent->tagValueConfirm.tagValueList;
1079+
p_tagValueList->nbPairs = nbElementsInPage;
1080+
p_tagValueList->pairs
1081+
= PIC(&p_content->content.tagValueConfirm.tagValueList.pairs[nextElementIdx]);
1082+
}
1083+
else {
1084+
// else display it as a TAG_VALUE_LIST
1085+
pageContent->type = TAG_VALUE_LIST;
1086+
nbgl_contentTagValueList_t *p_tagValueList = &pageContent->tagValueList;
1087+
p_tagValueList->nbPairs = nbElementsInPage;
1088+
p_tagValueList->pairs
1089+
= PIC(&p_content->content.tagValueConfirm.tagValueList.pairs[nextElementIdx]);
1090+
}
10721091
break;
10731092
case SWITCHES_LIST:
10741093
pageContent->switchesList.nbSwitches = nbElementsInPage;
@@ -1719,10 +1738,119 @@ static void keypadGenericUseCase(const char *title,
17191738
}
17201739
#endif
17211740

1722-
static uint8_t nbgl_useCaseGetNbPagesForContent(const nbgl_content_t *content,
1723-
uint8_t pageIdxStart,
1724-
bool isLast,
1725-
bool isSkippable)
1741+
/**
1742+
* @brief computes the number of tag/values pairs displayable in a page, with the given list of
1743+
* tag/value pairs
1744+
*
1745+
* @param nbPairs number of tag/value pairs to use in \b tagValueList
1746+
* @param tagValueList list of tag/value pairs
1747+
* @param startIndex first index to consider in \b tagValueList
1748+
* @param isSkippable if true, a skip header is added
1749+
* @param hasConfirmationButton if true, a confirmation button is added at last page
1750+
* @param requireSpecificDisplay (output) set to true if the tag/value needs a specific display:
1751+
* - centeredInfo flag is enabled
1752+
* - the tag/value doesn't fit in a page
1753+
* @return the number of tag/value pairs fitting in a page
1754+
*/
1755+
static uint8_t getNbTagValuesInPage(uint8_t nbPairs,
1756+
const nbgl_contentTagValueList_t *tagValueList,
1757+
uint8_t startIndex,
1758+
bool isSkippable,
1759+
bool hasConfirmationButton,
1760+
bool *requireSpecificDisplay)
1761+
{
1762+
uint8_t nbPairsInPage = 0;
1763+
uint16_t currentHeight = PRE_TAG_VALUE_MARGIN; // upper margin
1764+
uint16_t maxUsableHeight = TAG_VALUE_AREA_HEIGHT;
1765+
1766+
// if the review is skippable, it means that there is less height for tag/value pairs
1767+
// the small centering header becomes a touchable header
1768+
if (isSkippable) {
1769+
maxUsableHeight -= TOUCHABLE_HEADER_BAR_HEIGHT - SMALL_CENTERING_HEADER;
1770+
}
1771+
1772+
*requireSpecificDisplay = false;
1773+
while (nbPairsInPage < nbPairs) {
1774+
const nbgl_layoutTagValue_t *pair;
1775+
nbgl_font_id_e value_font;
1776+
uint16_t nbLines;
1777+
1778+
// margin between pairs
1779+
// 12 or 24 px between each tag/value pair
1780+
if (nbPairsInPage > 0) {
1781+
currentHeight += INTER_TAG_VALUE_MARGIN;
1782+
}
1783+
// fetch tag/value pair strings.
1784+
if (tagValueList->pairs != NULL) {
1785+
pair = PIC(&tagValueList->pairs[startIndex + nbPairsInPage]);
1786+
}
1787+
else {
1788+
pair = PIC(tagValueList->callback(startIndex + nbPairsInPage));
1789+
}
1790+
1791+
if (pair->forcePageStart && nbPairsInPage > 0) {
1792+
// This pair must be at the top of a page
1793+
break;
1794+
}
1795+
1796+
if (pair->centeredInfo) {
1797+
if (nbPairsInPage > 0) {
1798+
// This pair must be at the top of a page
1799+
break;
1800+
}
1801+
else {
1802+
// This pair is the only one of the page and has a specific display behavior
1803+
nbPairsInPage = 1;
1804+
*requireSpecificDisplay = true;
1805+
break;
1806+
}
1807+
}
1808+
1809+
// tag height
1810+
currentHeight += nbgl_getTextHeightInWidth(
1811+
SMALL_REGULAR_FONT, pair->item, AVAILABLE_WIDTH, tagValueList->wrapping);
1812+
// space between tag and value
1813+
currentHeight += 4;
1814+
// set value font
1815+
if (tagValueList->smallCaseForValue) {
1816+
value_font = SMALL_REGULAR_FONT;
1817+
}
1818+
else {
1819+
value_font = LARGE_MEDIUM_FONT;
1820+
}
1821+
// value height
1822+
currentHeight += nbgl_getTextHeightInWidth(
1823+
value_font, pair->value, AVAILABLE_WIDTH, tagValueList->wrapping);
1824+
// nb lines for value
1825+
nbLines = nbgl_getTextNbLinesInWidth(
1826+
value_font, pair->value, AVAILABLE_WIDTH, tagValueList->wrapping);
1827+
if ((currentHeight >= maxUsableHeight) || (nbLines > NB_MAX_LINES_IN_REVIEW)) {
1828+
if (nbPairsInPage == 0) {
1829+
// Pair too long to fit in a single screen
1830+
// It will be the only one of the page and has a specific display behavior
1831+
nbPairsInPage = 1;
1832+
*requireSpecificDisplay = true;
1833+
}
1834+
break;
1835+
}
1836+
nbPairsInPage++;
1837+
}
1838+
// if this is a TAG_VALUE_CONFIRM and we have reached the last pairs,
1839+
// let's check if it still fits with a CONFIRMATION button, and if not,
1840+
// remove the last pair
1841+
if (hasConfirmationButton && (nbPairsInPage == nbPairs)) {
1842+
maxUsableHeight -= UP_FOOTER_BUTTON_HEIGHT;
1843+
if (currentHeight > maxUsableHeight) {
1844+
nbPairsInPage--;
1845+
}
1846+
}
1847+
return nbPairsInPage;
1848+
}
1849+
1850+
static uint8_t getNbPagesForContent(const nbgl_content_t *content,
1851+
uint8_t pageIdxStart,
1852+
bool isLast,
1853+
bool isSkippable)
17261854
{
17271855
uint8_t nbElements = 0;
17281856
uint8_t nbPages = 0;
@@ -1737,8 +1865,16 @@ static uint8_t nbgl_useCaseGetNbPagesForContent(const nbgl_content_t *content,
17371865
// if the current page is not the first one (or last), a navigation bar exists
17381866
bool hasNav = !isLast || (pageIdxStart > 0) || (elemIdx > 0);
17391867
if (content->type == TAG_VALUE_LIST) {
1740-
nbElementsInPage = nbgl_useCaseGetNbTagValuesInPageExt(
1741-
nbElements, &content->content.tagValueList, elemIdx, isSkippable, &flag);
1868+
nbElementsInPage = getNbTagValuesInPage(
1869+
nbElements, &content->content.tagValueList, elemIdx, isSkippable, false, &flag);
1870+
}
1871+
else if (content->type == TAG_VALUE_CONFIRM) {
1872+
nbElementsInPage = getNbTagValuesInPage(nbElements,
1873+
&content->content.tagValueConfirm.tagValueList,
1874+
elemIdx,
1875+
isSkippable,
1876+
true,
1877+
&flag);
17421878
}
17431879
else if (content->type == INFOS_LIST) {
17441880
nbElementsInPage = nbgl_useCaseGetNbInfosInPage(
@@ -1782,10 +1918,10 @@ static uint8_t getNbPagesForGenericContents(const nbgl_genericContents_t *generi
17821918
if (p_content == NULL) {
17831919
return 0;
17841920
}
1785-
nbPages += nbgl_useCaseGetNbPagesForContent(p_content,
1786-
pageIdxStart + nbPages,
1787-
(i == (genericContents->nbContents - 1)),
1788-
isSkippable);
1921+
nbPages += getNbPagesForContent(p_content,
1922+
pageIdxStart + nbPages,
1923+
(i == (genericContents->nbContents - 1)),
1924+
isSkippable);
17891925
}
17901926

17911927
return nbPages;
@@ -2573,8 +2709,8 @@ uint8_t nbgl_useCaseGetNbTagValuesInPage(uint8_t nbPai
25732709
uint8_t startIndex,
25742710
bool *requireSpecificDisplay)
25752711
{
2576-
return nbgl_useCaseGetNbTagValuesInPageExt(
2577-
nbPairs, tagValueList, startIndex, false, requireSpecificDisplay);
2712+
return getNbTagValuesInPage(
2713+
nbPairs, tagValueList, startIndex, false, false, requireSpecificDisplay);
25782714
}
25792715

25802716
/**
@@ -2596,83 +2732,8 @@ uint8_t nbgl_useCaseGetNbTagValuesInPageExt(uint8_t nb
25962732
bool isSkippable,
25972733
bool *requireSpecificDisplay)
25982734
{
2599-
uint8_t nbPairsInPage = 0;
2600-
uint16_t currentHeight = PRE_TAG_VALUE_MARGIN; // upper margin
2601-
uint16_t maxUsableHeight = TAG_VALUE_AREA_HEIGHT;
2602-
2603-
// if the review is skippable, it means that there is less height for tag/value pairs
2604-
// the small centering header becomes a touchable header
2605-
if (isSkippable) {
2606-
maxUsableHeight -= TOUCHABLE_HEADER_BAR_HEIGHT - SMALL_CENTERING_HEADER;
2607-
}
2608-
2609-
*requireSpecificDisplay = false;
2610-
while (nbPairsInPage < nbPairs) {
2611-
const nbgl_layoutTagValue_t *pair;
2612-
nbgl_font_id_e value_font;
2613-
uint16_t nbLines;
2614-
2615-
// margin between pairs
2616-
// 12 or 24 px between each tag/value pair
2617-
if (nbPairsInPage > 0) {
2618-
currentHeight += INTER_TAG_VALUE_MARGIN;
2619-
}
2620-
// fetch tag/value pair strings.
2621-
if (tagValueList->pairs != NULL) {
2622-
pair = PIC(&tagValueList->pairs[startIndex + nbPairsInPage]);
2623-
}
2624-
else {
2625-
pair = PIC(tagValueList->callback(startIndex + nbPairsInPage));
2626-
}
2627-
2628-
if (pair->forcePageStart && nbPairsInPage > 0) {
2629-
// This pair must be at the top of a page
2630-
break;
2631-
}
2632-
2633-
if (pair->centeredInfo) {
2634-
if (nbPairsInPage > 0) {
2635-
// This pair must be at the top of a page
2636-
break;
2637-
}
2638-
else {
2639-
// This pair is the only one of the page and has a specific display behavior
2640-
nbPairsInPage = 1;
2641-
*requireSpecificDisplay = true;
2642-
break;
2643-
}
2644-
}
2645-
2646-
// tag height
2647-
currentHeight += nbgl_getTextHeightInWidth(
2648-
SMALL_REGULAR_FONT, pair->item, AVAILABLE_WIDTH, tagValueList->wrapping);
2649-
// space between tag and value
2650-
currentHeight += 4;
2651-
// set value font
2652-
if (tagValueList->smallCaseForValue) {
2653-
value_font = SMALL_REGULAR_FONT;
2654-
}
2655-
else {
2656-
value_font = LARGE_MEDIUM_FONT;
2657-
}
2658-
// value height
2659-
currentHeight += nbgl_getTextHeightInWidth(
2660-
value_font, pair->value, AVAILABLE_WIDTH, tagValueList->wrapping);
2661-
// nb lines for value
2662-
nbLines = nbgl_getTextNbLinesInWidth(
2663-
value_font, pair->value, AVAILABLE_WIDTH, tagValueList->wrapping);
2664-
if ((currentHeight >= maxUsableHeight) || (nbLines > NB_MAX_LINES_IN_REVIEW)) {
2665-
if (nbPairsInPage == 0) {
2666-
// Pair too long to fit in a single screen
2667-
// It will be the only one of the page and has a specific display behavior
2668-
nbPairsInPage = 1;
2669-
*requireSpecificDisplay = true;
2670-
}
2671-
break;
2672-
}
2673-
nbPairsInPage++;
2674-
}
2675-
return nbPairsInPage;
2735+
return getNbTagValuesInPage(
2736+
nbPairs, tagValueList, startIndex, isSkippable, false, requireSpecificDisplay);
26762737
}
26772738

26782739
/**
@@ -3013,7 +3074,7 @@ void nbgl_useCaseGenericSettings(const char *appName,
30133074
// fill navigation structure
30143075
uint8_t nbPages = getNbPagesForGenericContents(&genericContext.genericContents, 0, false);
30153076
if (infosList != NULL) {
3016-
nbPages += nbgl_useCaseGetNbPagesForContent(&FINISHING_CONTENT, nbPages, true, false);
3077+
nbPages += getNbPagesForContent(&FINISHING_CONTENT, nbPages, true, false);
30173078
}
30183079

30193080
prepareNavInfo(false, nbPages, NULL);

‎tests/screenshots/Makefile

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,8 @@ INC:= \
151151
$(PUBLIC_SDK_DIR)/include \
152152
$(PUBLIC_SDK_DIR)/lib_ux_nbgl \
153153
$(PUBLIC_SDK_DIR)/lib_cxng/include \
154+
$(PUBLIC_SDK_DIR)/io/include \
155+
$(PUBLIC_SDK_DIR)/io_legacy/include \
154156
$(NBGL_PATH)/include/fonts \
155157
$(NBGL_PATH)/include \
156158
$(PUBLIC_SDK_DIR)/qrcode/include \

0 commit comments

Comments
 (0)