From 1a145fe085b9d6736b29f902c69e99d5be11b6e5 Mon Sep 17 00:00:00 2001 From: pablohc Date: Mon, 20 Apr 2026 19:47:57 +0200 Subject: [PATCH] fix: keyboard feedback #1644 (#1697) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses reviewer feedback from #1644: - **Localize keyboard hint strings** — 13 hardcoded English strings replaced with \`tr()\` macro (\`STR_KB_HINT_*\`), making them translatable across all 22 languages (fallback to English when not yet translated) - **Deduplicate \`Lyra3CoversMetrics\`** — now derives from \`LyraMetrics\` via lambda copy, overriding only \`homeCoverTileHeight\` and \`homeRecentBooksCount\` (eliminates ~30 duplicated metric fields) - **Unify keyboard drawing in \`BaseTheme\`** — \`drawTextField\` and \`drawKeyboardKey\` overrides removed from \`LyraTheme\`; variability controlled via \`keyboardKeyCornerRadius\` metric (0=Base, 6=Lyra). Unified text field padding to 6, adopted Lyra's secondary label draw order (main first, then secondary) - **Add URL-optimized keyboard layout** — \`urlLayout\` with \`:\` and \`/\` replacing \`=\` and \`,\` for easier URL input without switching to SYM mode --- ### AI Usage While CrossPoint doesn't have restrictions on AI tools in contributing, please be transparent about their usage as it helps set the right context for reviewers. Did you use AI tools to help write this code? _** YES **_ --- lib/I18n/translations/english.yaml | 15 +++++ src/activities/util/KeyboardEntryActivity.cpp | 48 +++++++------- src/activities/util/KeyboardEntryActivity.h | 43 ++++++++++++ src/components/themes/BaseTheme.cpp | 36 +++++++--- src/components/themes/BaseTheme.h | 4 +- src/components/themes/lyra/Lyra3CoversTheme.h | 44 ++----------- src/components/themes/lyra/LyraTheme.cpp | 65 ------------------- src/components/themes/lyra/LyraTheme.h | 8 +-- 8 files changed, 123 insertions(+), 140 deletions(-) diff --git a/lib/I18n/translations/english.yaml b/lib/I18n/translations/english.yaml index af66e074c..c7db65088 100644 --- a/lib/I18n/translations/english.yaml +++ b/lib/I18n/translations/english.yaml @@ -298,3 +298,18 @@ STR_CRASH_TITLE: "System Crash" STR_CRASH_DESCRIPTION: "A detailed report was saved to crash_report.txt. Please include this file in your bug report." STR_CRASH_REASON: "Crash reason:" STR_CRASH_NO_REASON: "(No reason was recorded)" +STR_KB_HINT_MOVE_CURSOR: "Press LEFT or RIGHT to move cursor" +STR_KB_HINT_RETURN_CURSOR: "Press LEFT to return to cursor position" +STR_KB_HINT_HIDE_PASSWORD: "Hold RIGHT then press [***] to hide password" +STR_KB_HINT_SHOW_PASSWORD: "Hold RIGHT then press [abc] to show password" +STR_KB_HINT_TOGGLE_HIDE_PASSWORD: "Press [***] to hide password" +STR_KB_HINT_TOGGLE_SHOW_PASSWORD: "Press [abc] to show password" +STR_KB_HINT_EDIT_ENTRY: "Hold UP to edit entry" +STR_KB_TIPS: "Tips:" +STR_KB_HINT_RETURN_KEYBOARD: "Press DOWN to return to keyboard" +STR_KB_HINT_EXIT_URL_MODE: "Press ABC to exit URL mode" +STR_KB_HINT_CLEAR_TEXT: "Hold DEL to clear all text" +STR_KB_HINT_SECONDARY_CHAR: "Hold SELECT for secondary char" +STR_KB_HINT_UPPER_SECONDARY: "Hold SELECT for UPPERCASE or secondary char" +STR_KB_HINT_LOWER_SECONDARY: "Hold SELECT for lowercase or secondary char" +STR_KB_HINT_URL_SNIPPETS: "Press URL for snippets" diff --git a/src/activities/util/KeyboardEntryActivity.cpp b/src/activities/util/KeyboardEntryActivity.cpp index 7f25f6857..eebd0b5d8 100644 --- a/src/activities/util/KeyboardEntryActivity.cpp +++ b/src/activities/util/KeyboardEntryActivity.cpp @@ -49,7 +49,7 @@ int KeyboardEntryActivity::getTotalRowCount() const { return getContentRowCount( bool KeyboardEntryActivity::isBottomRow(const int row) const { return row == getContentRowCount(); } char KeyboardEntryActivity::getSelectedChar() const { - const KeyDef(*layout)[COLS] = symMode ? symLayout : abcLayout; + const KeyDef(*layout)[COLS] = symMode ? symLayout : (inputType == InputType::Url ? urlLayout : abcLayout); if (selectedRow < 0 || selectedRow >= getContentRowCount()) return '\0'; if (selectedCol < 0 || selectedCol >= COLS) return '\0'; @@ -526,20 +526,22 @@ void KeyboardEntryActivity::render(RenderLock&&) { const int hintY = underlineY + 4; if (cursorMode) { int hintLineY = hintY; - renderer.drawCenteredText(SMALL_FONT_ID, hintLineY, "Press < or > to move cursor", true); - hintLineY += hintLh; - if (inputType == InputType::Password) { - const char* passTip; - if (togglePos) { - passTip = "Press < to return to cursor position"; - } else { - passTip = - passwordVisible ? "Hold > then press [***] to hide password" : "Hold > then press [abc] to show password"; + if (inputType == InputType::Password && togglePos) { + renderer.drawCenteredText( + SMALL_FONT_ID, hintLineY, + passwordVisible ? tr(STR_KB_HINT_TOGGLE_HIDE_PASSWORD) : tr(STR_KB_HINT_TOGGLE_SHOW_PASSWORD), true); + hintLineY += hintLh; + renderer.drawCenteredText(SMALL_FONT_ID, hintLineY, tr(STR_KB_HINT_RETURN_CURSOR), true); + } else { + renderer.drawCenteredText(SMALL_FONT_ID, hintLineY, tr(STR_KB_HINT_MOVE_CURSOR), true); + hintLineY += hintLh; + if (inputType == InputType::Password) { + const char* passTip = passwordVisible ? tr(STR_KB_HINT_HIDE_PASSWORD) : tr(STR_KB_HINT_SHOW_PASSWORD); + renderer.drawCenteredText(SMALL_FONT_ID, hintLineY, passTip, true); } - renderer.drawCenteredText(SMALL_FONT_ID, hintLineY, passTip, true); } } else { - renderer.drawCenteredText(SMALL_FONT_ID, hintY, "Hold UP to edit entry", true); + renderer.drawCenteredText(SMALL_FONT_ID, hintY, tr(STR_KB_HINT_EDIT_ENTRY), true); } } @@ -575,37 +577,37 @@ void KeyboardEntryActivity::render(RenderLock&&) { if (tipCount > 0) { int y = (underlineBottom + keyboardStartY) / 2 - (tipCount + 1) * tipsLh / 2; - drawTip("Tips:", y); + drawTip(tr(STR_KB_TIPS), y); y += tipsLh; if (cursorMode) { - drawTip("Press DOWN to return to keyboard", y); + drawTip(tr(STR_KB_HINT_RETURN_KEYBOARD), y); } else if (urlMode) { - drawTip("Press ABC to exit URL mode", y); + drawTip(tr(STR_KB_HINT_EXIT_URL_MODE), y); y += tipsLh; if (!text.empty()) { - drawTip("Hold DEL to clear all text", y); + drawTip(tr(STR_KB_HINT_CLEAR_TEXT), y); } } else if (symMode) { if (!text.empty()) { - drawTip("Hold DEL to clear all text", y); + drawTip(tr(STR_KB_HINT_CLEAR_TEXT), y); } } else { const char* altCharTip; if (inputType == InputType::Url) { - altCharTip = "Hold SELECT for secondary char"; + altCharTip = tr(STR_KB_HINT_SECONDARY_CHAR); } else if (shiftState > 0) { - altCharTip = "Hold SELECT for lowercase or secondary char"; + altCharTip = tr(STR_KB_HINT_LOWER_SECONDARY); } else { - altCharTip = "Hold SELECT for UPPERCASE or secondary char"; + altCharTip = tr(STR_KB_HINT_UPPER_SECONDARY); } drawTip(altCharTip, y); y += tipsLh; if (inputType == InputType::Url) { - drawTip("Press URL for snippets", y); + drawTip(tr(STR_KB_HINT_URL_SNIPPETS), y); y += tipsLh; } if (!text.empty()) { - drawTip("Hold DEL to clear all text", y); + drawTip(tr(STR_KB_HINT_CLEAR_TEXT), y); } } } @@ -625,7 +627,7 @@ void KeyboardEntryActivity::render(RenderLock&&) { urlLeftMargin = urlCenterX - urlTotalWidth / 2; } - const KeyDef(*layout)[COLS] = symMode ? symLayout : abcLayout; + const KeyDef(*layout)[COLS] = symMode ? symLayout : (inputType == InputType::Url ? urlLayout : abcLayout); const int contentRows = getContentRowCount(); for (int row = 0; row < contentRows; row++) { diff --git a/src/activities/util/KeyboardEntryActivity.h b/src/activities/util/KeyboardEntryActivity.h index c6b659cb3..0a86febf8 100644 --- a/src/activities/util/KeyboardEntryActivity.h +++ b/src/activities/util/KeyboardEntryActivity.h @@ -125,6 +125,49 @@ class KeyboardEntryActivity : public Activity { {',', '<'}}, }; + static constexpr KeyDef urlLayout[ABC_ROWS][COLS] = { + {{'1', '!'}, + {'2', '@'}, + {'3', '#'}, + {'4', '$'}, + {'5', '%'}, + {'6', '^'}, + {'7', '&'}, + {'8', '*'}, + {'9', '('}, + {'0', ')'}}, + {{'q', 'Q'}, + {'w', 'W'}, + {'e', 'E'}, + {'r', 'R'}, + {'t', 'T'}, + {'y', 'Y'}, + {'u', 'U'}, + {'i', 'I'}, + {'o', 'O'}, + {'p', 'P'}}, + {{'a', 'A'}, + {'s', 'S'}, + {'d', 'D'}, + {'f', 'F'}, + {'g', 'G'}, + {'h', 'H'}, + {'j', 'J'}, + {'k', 'K'}, + {'l', 'L'}, + {'-', '_'}}, + {{'z', 'Z'}, + {'x', 'X'}, + {'c', 'C'}, + {'v', 'V'}, + {'b', 'B'}, + {'n', 'N'}, + {'m', 'M'}, + {':', '+'}, + {'.', '>'}, + {'/', '<'}}, + }; + static constexpr KeyDef symLayout[SYM_ROWS][COLS] = { {{'1', '\0'}, {'2', '\0'}, diff --git a/src/components/themes/BaseTheme.cpp b/src/components/themes/BaseTheme.cpp index d02268a45..49f5d2a7d 100644 --- a/src/components/themes/BaseTheme.cpp +++ b/src/components/themes/BaseTheme.cpp @@ -783,13 +783,14 @@ void BaseTheme::drawHelpText(const GfxRenderer& renderer, Rect rect, const char* void BaseTheme::drawTextField(const GfxRenderer& renderer, Rect rect, const int textWidth, bool cursorMode, int contentStartX, int contentWidth) const { + const auto& metrics = UITheme::getInstance().getMetrics(); const int lineHeight = renderer.getLineHeight(UI_12_FONT_ID); - const int lineY = rect.y + rect.height + lineHeight + BaseMetrics::values.verticalSpacing; + const int lineY = rect.y + rect.height + lineHeight + metrics.verticalSpacing; const int thickness = cursorMode ? 3 : 1; if (contentWidth > 0) { renderer.drawLine(rect.x + contentStartX, lineY, rect.x + contentStartX + contentWidth, lineY, thickness, true); } else { - const int hPadding = 4; + const int hPadding = 6; const int lineW = textWidth + hPadding * 2; renderer.drawLine(rect.x + (rect.width - lineW) / 2, lineY, rect.x + (rect.width + lineW) / 2, lineY, thickness, true); @@ -799,18 +800,37 @@ void BaseTheme::drawTextField(const GfxRenderer& renderer, Rect rect, const int void BaseTheme::drawKeyboardKey(const GfxRenderer& renderer, Rect rect, const char* label, const bool isSelected, const char* secondaryLabel, const KeyboardKeyType keyType, const bool inactiveSelection) const { + const auto& metrics = UITheme::getInstance().getMetrics(); + const int cr = metrics.keyboardKeyCornerRadius; + if (isSelected) { if (inactiveSelection) { - renderer.drawRect(rect.x, rect.y, rect.width, rect.height, 2, true); + if (cr > 0) { + renderer.fillRoundedRect(rect.x, rect.y, rect.width, rect.height, cr, Color::LightGray); + } else { + renderer.drawRect(rect.x, rect.y, rect.width, rect.height, 2, true); + } } else if (keyType == KeyboardKeyType::Disabled) { - renderer.fillRectDither(rect.x, rect.y, rect.width, rect.height, Color::LightGray); + if (cr > 0) { + renderer.fillRoundedRect(rect.x, rect.y, rect.width, rect.height, cr, Color::LightGray); + } else { + renderer.fillRectDither(rect.x, rect.y, rect.width, rect.height, Color::LightGray); + } } else { - renderer.fillRect(rect.x, rect.y, rect.width, rect.height, true); + if (cr > 0) { + renderer.fillRoundedRect(rect.x, rect.y, rect.width, rect.height, cr, Color::Black); + } else { + renderer.fillRect(rect.x, rect.y, rect.width, rect.height, true); + } } } else if (keyType == KeyboardKeyType::Shift || keyType == KeyboardKeyType::Mode || keyType == KeyboardKeyType::Del || keyType == KeyboardKeyType::Space || keyType == KeyboardKeyType::Ok || keyType == KeyboardKeyType::Disabled) { - renderer.drawRect(rect.x, rect.y, rect.width, rect.height); + if (cr > 0) { + renderer.drawRoundedRect(rect.x, rect.y, rect.width, rect.height, 1, cr, true); + } else { + renderer.drawRect(rect.x, rect.y, rect.width, rect.height); + } } const bool invert = isSelected && !inactiveSelection; @@ -841,10 +861,10 @@ void BaseTheme::drawKeyboardKey(const GfxRenderer& renderer, Rect rect, const ch const int textX = rect.x + (rect.width - itemWidth) / 2; const int textY = rect.y + (rect.height - renderer.getLineHeight(UI_12_FONT_ID)) / 2; + renderer.drawText(UI_12_FONT_ID, textX, textY, label, !invert); + if (hasSecondary) { const int secWidth = renderer.getTextWidth(SMALL_FONT_ID, secondaryLabel); renderer.drawText(SMALL_FONT_ID, rect.x + rect.width - secWidth - 1, rect.y, secondaryLabel, !invert); } - - renderer.drawText(UI_12_FONT_ID, textX, textY, label, !invert); } diff --git a/src/components/themes/BaseTheme.h b/src/components/themes/BaseTheme.h index 512b9f867..6dd462e7d 100644 --- a/src/components/themes/BaseTheme.h +++ b/src/components/themes/BaseTheme.h @@ -67,6 +67,7 @@ struct ThemeMetrics { int keyboardVerticalOffset; int keyboardTextFieldWidthPercent; int keyboardWidthPercent; + int keyboardKeyCornerRadius; }; enum UIIcon { Folder, Text, Image, Book, File, Recent, Settings, Transfer, Library, Wifi, Hotspot }; @@ -111,7 +112,8 @@ constexpr ThemeMetrics values = {.batteryWidth = 15, .keyboardCenteredText = false, .keyboardVerticalOffset = -13, .keyboardTextFieldWidthPercent = 85, - .keyboardWidthPercent = 90}; + .keyboardWidthPercent = 90, + .keyboardKeyCornerRadius = 0}; } class BaseTheme { diff --git a/src/components/themes/lyra/Lyra3CoversTheme.h b/src/components/themes/lyra/Lyra3CoversTheme.h index 5d2bd9740..b17faed6d 100644 --- a/src/components/themes/lyra/Lyra3CoversTheme.h +++ b/src/components/themes/lyra/Lyra3CoversTheme.h @@ -6,44 +6,14 @@ class GfxRenderer; -// Lyra theme metrics (zero runtime cost) namespace Lyra3CoversMetrics { -constexpr ThemeMetrics values = {.batteryWidth = 16, - .batteryHeight = 12, - .topPadding = 5, - .batteryBarHeight = 40, - .headerHeight = 84, - .verticalSpacing = 16, - .contentSidePadding = 20, - .listRowHeight = 40, - .listWithSubtitleRowHeight = 60, - .menuRowHeight = 64, - .menuSpacing = 8, - .tabSpacing = 8, - .tabBarHeight = 40, - .scrollBarWidth = 4, - .scrollBarRightOffset = 5, - .homeTopPadding = 56, - .homeCoverHeight = 226, - .homeCoverTileHeight = 300, - .homeRecentBooksCount = 3, - .buttonHintsHeight = 40, - .sideButtonHintsWidth = 30, - .progressBarHeight = 16, - .progressBarMarginTop = 1, - .statusBarHorizontalMargin = 5, - .statusBarVerticalMargin = 19, - .keyboardKeyWidth = 31, - .keyboardKeyHeight = 40, - .keyboardKeySpacing = 0, - .keyboardBottomKeyHeight = 35, - .keyboardBottomKeySpacing = 5, - .keyboardBottomAligned = true, - .keyboardCenteredText = false, - .keyboardVerticalOffset = -7, - .keyboardTextFieldWidthPercent = 85, - .keyboardWidthPercent = 90}; -} +constexpr ThemeMetrics values = [] { + ThemeMetrics v = LyraMetrics::values; + v.homeCoverTileHeight = 300; + v.homeRecentBooksCount = 3; + return v; +}(); +} // namespace Lyra3CoversMetrics class Lyra3CoversTheme : public LyraTheme { public: diff --git a/src/components/themes/lyra/LyraTheme.cpp b/src/components/themes/lyra/LyraTheme.cpp index a28349ef1..73a898b9a 100644 --- a/src/components/themes/lyra/LyraTheme.cpp +++ b/src/components/themes/lyra/LyraTheme.cpp @@ -577,68 +577,3 @@ void LyraTheme::fillPopupProgress(const GfxRenderer& renderer, const Rect& layou renderer.displayBuffer(HalDisplay::FAST_REFRESH); } - -void LyraTheme::drawTextField(const GfxRenderer& renderer, Rect rect, const int textWidth, bool cursorMode, - int contentStartX, int contentWidth) const { - int lineY = rect.y + rect.height + renderer.getLineHeight(UI_12_FONT_ID) + LyraMetrics::values.verticalSpacing; - const int thickness = cursorMode ? 3 : 1; - if (contentWidth > 0) { - renderer.drawLine(rect.x + contentStartX, lineY, rect.x + contentStartX + contentWidth, lineY, thickness, true); - } else { - int lineW = textWidth + hPaddingInSelection * 2; - renderer.drawLine(rect.x + (rect.width - lineW) / 2, lineY, rect.x + (rect.width + lineW) / 2, lineY, thickness, - true); - } -} - -void LyraTheme::drawKeyboardKey(const GfxRenderer& renderer, Rect rect, const char* label, const bool isSelected, - const char* secondaryLabel, const KeyboardKeyType keyType, - const bool inactiveSelection) const { - if (isSelected) { - if (inactiveSelection) { - renderer.fillRoundedRect(rect.x, rect.y, rect.width, rect.height, cornerRadius, Color::LightGray); - } else if (keyType == KeyboardKeyType::Disabled) { - renderer.fillRoundedRect(rect.x, rect.y, rect.width, rect.height, cornerRadius, Color::LightGray); - } else { - renderer.fillRoundedRect(rect.x, rect.y, rect.width, rect.height, cornerRadius, Color::Black); - } - } else if (keyType == KeyboardKeyType::Shift || keyType == KeyboardKeyType::Mode || keyType == KeyboardKeyType::Del || - keyType == KeyboardKeyType::Space || keyType == KeyboardKeyType::Ok || - keyType == KeyboardKeyType::Disabled) { - renderer.drawRoundedRect(rect.x, rect.y, rect.width, rect.height, 1, cornerRadius, true); - } - - const bool invert = isSelected && !inactiveSelection; - - if (keyType == KeyboardKeyType::Space) { - const int lineHalfWidth = rect.width * 3 / 10; - const int centerX = rect.x + rect.width / 2; - const int lineY = rect.y + rect.height / 2 + 3; - renderer.drawLine(centerX - lineHalfWidth, lineY, centerX + lineHalfWidth, lineY, 3, !invert); - return; - } - - if (keyType == KeyboardKeyType::Del) { - const int centerX = rect.x + rect.width / 2; - const int centerY = rect.y + rect.height / 2; - const int arrowLen = rect.width / 4; - const int arrowHead = arrowLen / 2; - renderer.drawLine(centerX - arrowLen / 2, centerY, centerX + arrowLen / 2, centerY, 3, !invert); - renderer.drawLine(centerX - arrowLen / 2, centerY, centerX - arrowLen / 2 + arrowHead, centerY - arrowHead, 3, - !invert); - renderer.drawLine(centerX - arrowLen / 2, centerY, centerX - arrowLen / 2 + arrowHead, centerY + arrowHead, 3, - !invert); - return; - } - - const bool hasSecondary = secondaryLabel != nullptr && secondaryLabel[0] != '\0'; - const int textWidth = renderer.getTextWidth(UI_12_FONT_ID, label); - const int textX = rect.x + (rect.width - textWidth) / 2; - const int textY = rect.y + (rect.height - renderer.getLineHeight(UI_12_FONT_ID)) / 2; - renderer.drawText(UI_12_FONT_ID, textX, textY, label, !invert); - - if (hasSecondary) { - const int secWidth = renderer.getTextWidth(SMALL_FONT_ID, secondaryLabel); - renderer.drawText(SMALL_FONT_ID, rect.x + rect.width - secWidth - 1, rect.y, secondaryLabel, !invert); - } -} diff --git a/src/components/themes/lyra/LyraTheme.h b/src/components/themes/lyra/LyraTheme.h index b98e52974..c539cf6b9 100644 --- a/src/components/themes/lyra/LyraTheme.h +++ b/src/components/themes/lyra/LyraTheme.h @@ -40,7 +40,8 @@ constexpr ThemeMetrics values = {.batteryWidth = 16, .keyboardCenteredText = false, .keyboardVerticalOffset = -7, .keyboardTextFieldWidthPercent = 85, - .keyboardWidthPercent = 90}; + .keyboardWidthPercent = 90, + .keyboardKeyCornerRadius = 6}; } class LyraTheme : public BaseTheme { @@ -71,10 +72,5 @@ class LyraTheme : public BaseTheme { void drawEmptyRecents(const GfxRenderer& renderer, const Rect rect) const; Rect drawPopup(const GfxRenderer& renderer, const char* message) const override; void fillPopupProgress(const GfxRenderer& renderer, const Rect& layout, const int progress) const override; - void drawTextField(const GfxRenderer& renderer, Rect rect, const int textWidth, bool cursorMode = false, - int contentStartX = 0, int contentWidth = 0) const override; - void drawKeyboardKey(const GfxRenderer& renderer, Rect rect, const char* label, const bool isSelected, - const char* secondaryLabel = nullptr, KeyboardKeyType keyType = KeyboardKeyType::Normal, - bool inactiveSelection = false) const override; bool showsFileIcons() const override { return true; } };