Repository navigation
Conversation
Adds window_manager_get_windows() (bounded, no LVGL locks) and window_manager_raise(), which moves a buried window to the top and rebuilds its widgets through the app's create_widgets callback. The app keeps running, so its own state survives - that is what home and the recents switcher need. Raising takes the lifecycle mutex, so it is documented as not callable from an LVGL event callback; callers defer it to the main dispatcher. Also adds app_manager_get_instance_app_id(), mirroring the topmost lookup, so any window's app can be identified without assuming it is on top.
The apps mode and the settings mode each carried their own icon table. One lookup now serves both, and the recents switcher shows the same icon an app has in the launcher instead of a third table. The launcher gets an icon of its own, which only the switcher uses (it stays hidden from the app list).
Swipe up from the bottom edge raises the launcher; keep holding it for 500 ms and the recents switcher opens. Swipe right from the left edge closes the topmost app, like the toolbar close, and does nothing on the launcher. Only drags that start in a 20 px edge strip belong to the system, and only when they travel 40 px in the expected direction, so paging, scrolling and carousels inside apps keep working. The release is consumed, so the widget under the finger does not also get a click. The left edge is left alone when the system bars use the side layout, and the statusbar keeps owning the top edge. The hold timer only exists while a bottom-edge press is down, the edge scan uses fixed-size stack arrays, and a shown overlay disables the gestures. Gate everything behind CONFIG_TT_SYSTEM_GESTURES (default on) and require a pointer device at runtime, so keypad/encoder-only devices are untouched. The gesture geometry is a pure function, covered by a unit test.
The panel now hangs from the top edge, full width, and slides in over 180 ms (the theme's animation setting is respected). Its existing toggles are joined by a clock, the Wi-Fi status (off / on / network name), a recents tile, and a brightness slider when the display has a backlight that can be dimmed. Brightness applies immediately while dragging and is persisted once when the panel closes, on the main dispatcher, so dragging does not write a settings file per step. Boards whose devicetree disables the backlight (es3c28p today) simply do not get the slider.
Correctness: - Back (and the toolbar's default close) now act on the topmost window's app instead of the newest app instance. After going home the launcher is on top while a newer instance is buried below it, so the old lookup could close the wrong app. - The brightness slider requires a dimmable backlight; a 0-1 on/off range no longer gets a slider in which every position means "on". - The launcher lookup walks the whole window stack instead of stopping at 16 entries. - A press whose release never arrives (e.g. the display was reset mid-press) can no longer leave the gesture state armed: the hold timer and the next press recover from lv_indev_get_state(). Single-sourcing: - One system-window helper: window iteration, top-window lookup, id<->context casts and the deferred raise, so the lock-order reason lives in one place. - One backlight helper shared by the Display app and the quick panel (lookup, dimmable test, percentage mapping). - WINDOW_ID_NONE names the "no window" id; the new app-manager lookup is in the module symbol table.
feat: iOS-style system gestures (home, back, recents, quick panel)
New from-scratch redesign of the nine front-end screens for the ES3C28P (240x320): HTML/CSS sources, PNGs at 1x and 2x, contact sheet, and SPEC.md with palette, type scale, radii, touch targets, per-screen LVGL constraints, states, transitions, and a measured flash/RAM estimate (~41 KB flash, ~40 KB peak UI RAM). Supersedes docs/design/, which is left untouched for reference. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Walkthrough
Priority: Merge Risk:
 |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 28.17% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 71 functions across 21 files. (13 skipped… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly identifies the redesign mockups, which are a major part of the pull request. It is concise and related to the documented design work, although it does not mention the accompanying fi… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Title check
-
Explanation
The title clearly identifies the redesign mockups, which are a major part of the pull request. It is concise and related to the documented design work, although it does not mention the accompanying firmware changes.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 28.17% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 71 functions across 21 files. (13 skipped: 13 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
-
-
- Source mode: Auto
- Repository: Unavailable
- Branch: Unavailable
- File: No configuration file from this repository applied
- Commit: Unavailable
- Layers: Organization UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
7e2842b3-ab94-4ef6-abc8-013a1273e520
-
docs/design-v2/png/1x/dialog.pngis excluded by!**/*.pngdocs/design-v2/png/1x/files.pngis excluded by!**/*.pngdocs/design-v2/png/1x/home.pngis excluded by!**/*.pngdocs/design-v2/png/1x/keyboard.pngis excluded by!**/*.pngdocs/design-v2/png/1x/lock.pngis excluded by!**/*.pngdocs/design-v2/png/1x/quick.pngis excluded by!**/*.pngdocs/design-v2/png/1x/recents.pngis excluded by!**/*.pngdocs/design-v2/png/1x/settings.pngis excluded by!**/*.pngdocs/design-v2/png/1x/wifi.pngis excluded by!**/*.pngdocs/design-v2/png/2x/dialog.pngis excluded by!**/*.pngdocs/design-v2/png/2x/files.pngis excluded by!**/*.pngdocs/design-v2/png/2x/home.pngis excluded by!**/*.pngdocs/design-v2/png/2x/keyboard.pngis excluded by!**/*.pngdocs/design-v2/png/2x/lock.pngis excluded by!**/*.pngdocs/design-v2/png/2x/quick.pngis excluded by!**/*.pngdocs/design-v2/png/2x/recents.pngis excluded by!**/*.pngdocs/design-v2/png/2x/settings.pngis excluded by!**/*.pngdocs/design-v2/png/2x/wifi.pngis excluded by!**/*.pngdocs/design-v2/png/contact-sheet.pngis excluded by!**/*.pngdocs/design-v2/png/contact-sheet@2x.pngis excluded by!**/*.pngdocs/design-v2/src/fonts/inter-latin-400-normal.woff2is excluded by!**/*.woff2docs/design-v2/src/fonts/inter-latin-500-normal.woff2is excluded by!**/*.woff2docs/design/mockups/fonts/inter-latin-400-normal.woff2is excluded by!**/*.woff2docs/design/mockups/fonts/inter-latin-500-normal.woff2is excluded by!**/*.woff2docs/design/png/1x/app.pngis excluded by!**/*.pngdocs/design/png/1x/dialog.pngis excluded by!**/*.pngdocs/design/png/1x/home.pngis excluded by!**/*.pngdocs/design/png/1x/keyboard.pngis excluded by!**/*.pngdocs/design/png/1x/lock.pngis excluded by!**/*.pngdocs/design/png/1x/quick-panel.pngis excluded by!**/*.pngdocs/design/png/1x/recents.pngis excluded by!**/*.pngdocs/design/png/1x/settings.pngis excluded by!**/*.pngdocs/design/png/1x/wifi.pngis excluded by!**/*.pngdocs/design/png/2x/app.pngis excluded by!**/*.pngdocs/design/png/2x/dialog.pngis excluded by!**/*.pngdocs/design/png/2x/home.pngis excluded by!**/*.pngdocs/design/png/2x/keyboard.pngis excluded by!**/*.pngdocs/design/png/2x/lock.pngis excluded by!**/*.pngdocs/design/png/2x/quick-panel.pngis excluded by!**/*.pngdocs/design/png/2x/recents.pngis excluded by!**/*.pngdocs/design/png/2x/settings.pngis excluded by!**/*.pngdocs/design/png/2x/wifi.pngis excluded by!**/*.pngdocs/design/png/contact-sheet.pngis excluded by!**/*.png
-
Modules/app-module/include/app/manager.hModules/app-module/source/manager.cppModules/app-module/source/module.cppModules/lvgl-window-manager-module/include/lvgl_window_manager/window_manager.hModules/lvgl-window-manager-module/source/module.cppModules/lvgl-window-manager-module/source/window_manager.cppTactility/KconfigTactility/Private/Tactility/Backlight.hTactility/Private/Tactility/app/AppIcon.hTactility/Private/Tactility/lvgl/SystemGestures.hTactility/Private/Tactility/lvgl/SystemRecents.hTactility/Private/Tactility/lvgl/SystemWindow.hTactility/Source/app/AppIcon.cppTactility/Source/app/display/Display.cppTactility/Source/app/launcher/AppsMode.cppTactility/Source/app/launcher/SettingsMode.cppTactility/Source/lvgl/Lvgl.cppTactility/Source/lvgl/SystemGestures.cppTactility/Source/lvgl/SystemMenu.cppTactility/Source/lvgl/SystemRecents.cppTactility/Tests/Source/SystemGesturesTest.cppdocs/design-v2/SPEC.mddocs/design-v2/png/checks.jsondocs/design-v2/render.mjsdocs/design-v2/src/index.htmldocs/design-v2/src/styles.cssdocs/design/mockups/contact-sheet.htmldocs/design/mockups/index.htmldocs/design/mockups/styles.cssdocs/design/redesign-spec.mddocs/mobile/ios-gestures-retro.mddocs/mobile/ios-gestures-review.mddocs/mobile/ios-gestures-spec.mddocs/mobile/ios-gestures-tickets.md
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.
-
|
|
||
| | Fonte | Subconjunto | Glifos | Flash | | ||
| |---|---|---|---| | ||
| | Inter Regular 12 | ASCII 0x20–0x7E | 95 | 4,0 KB | |
There was a problem hiding this comment.
Add non-ASCII UI glyphs to the font subsets.
The ASCII-only subsets omit · from the footer and keyboard hint, … from “Scanning…”, and • from the password field. The proposed Latin-1 extension would not add the bullet or ellipsis. Specify these glyphs explicitly before using the font estimates as an implementation budget.
Also applies to: 277-277, 279-279
|
|
||
| | Asset | Size / weight | Subset | Glyphs | Est. flash | | ||
| |---|---|---|---|---| | ||
| | Caption | 12 / 400 | ASCII | 95 | 6.0 KB | |
There was a problem hiding this comment.
Include the glyphs used by the proposed screens.
The 12 px ASCII subset excludes the footer’s ·. The 13 px ASCII subset excludes … in “Scanning…”. The 15 px Latin-1 subset also excludes the password bullet •. Add these glyphs to the specified bitmap subsets, or change the displayed strings before font conversion. Otherwise, the proposed UI cannot render those strings as shown.
Also applies to: 261-261, 262-262
| const size_t count = window_manager_get_windows(windows, offset, CHUNK_SIZE); | ||
| for (size_t index = 0; index < count && index < CHUNK_SIZE; index++) { |
There was a problem hiding this comment.
Fix forEachWindow(): it visits stale entries in the last partial chunk.
window_manager_get_windows() returns the total window count, not the number of entries it copied. The inner loop bound index < count && index < CHUNK_SIZE therefore equals CHUNK_SIZE for every chunk when the stack has more than 8 windows.
Example: with 10 windows, the chunk at offset == 8 copies only 2 entries. The loop still passes windows[2] to windows[7] to action. Those slots hold entries left over from the previous chunk, so the action sees duplicate windows.
findLauncherWindow() and findAppInstance() tolerate duplicates. Any other caller of forEachWindow() would see repeated windows.
Also, if the stack shrinks between the two calls, count can be smaller than offset. The fix must not underflow in that case.
🐛 Proposed fix
-
--- "a/Tactility/Private/Tactility/lvgl/SystemWindow.h" +++ "b/Tactility/Private/Tactility/lvgl/SystemWindow.h" @@ -30,8 +30,10 @@ WindowInfo windows[CHUNK_SIZE]; const size_t total = window_manager_get_windows(nullptr, 0, 0); for (size_t offset = 0; offset < total; offset += CHUNK_SIZE) { - const size_t count = window_manager_get_windows(windows, offset, CHUNK_SIZE); - for (size_t index = 0; index < count && index < CHUNK_SIZE; index++) { + const size_t current_total = window_manager_get_windows(windows, offset, CHUNK_SIZE); + const size_t available = current_total > offset ? current_total - offset : 0; + const size_t count = available < CHUNK_SIZE ? available : CHUNK_SIZE; + for (size_t index = 0; index < count; index++) { if (!action(windows[index])) { return; }
Committable suggestion
-
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.Suggested changeconst size_t count = window_manager_get_windows(windows, offset, CHUNK_SIZE); for (size_t index = 0; index < count && index < CHUNK_SIZE; index++) { const size_t current_total = window_manager_get_windows(windows, offset, CHUNK_SIZE); const size_t available = current_total > offset ? current_total - offset : 0; const size_t count = available < CHUNK_SIZE ? available : CHUNK_SIZE; for (size_t index = 0; index < count; index++) {
| if (isPressStillDown()) { | ||
| // One press at a time: a second finger is ignored | ||
| return; | ||
| } |
There was a problem hiding this comment.
🔎 Supported by static analysis
-
Script executed:
sed -n '90,215p' Tactility/Source/lvgl/SystemGestures.cpp rg -n 'lv_indev_wait_release|LV_EVENT_RELEASED|isPressStillDown|clearPress' Tactility/Source/lvgl/SystemGestures.cpp
Repository: TactilityProject/Tactility
Length of output: 4575
Script executed:
set +e printf '%s\n' '--- bound LVGL symbols and files ---' rg -n -F --glob '!build/**' --glob '!dist/**' -e 'lv_indev_wait_release' -e 'LV_EVENT_RELEASED' -e 'lv_indev_send_event' -e 'LV_INDEV_STATE_PRESSED' -e 'LV_INDEV_STATE_RELEASED' . printf '%s\n' '--- candidate LVGL files ---' rg --files -g '*lv_indev*' -g '*lv_event*' -g '*lvgl*' | sed -n '1,160p' printf '%s\n' '--- SystemGestures registration and lifecycle ---' sed -n '205,270p' Tactility/Source/lvgl/SystemGestures.cpp printf '%s\n' '--- dependency/submodule declarations ---' git submodule status rg -n -F --glob 'CMakeLists.txt' --glob '*.cmake' --glob '*.yml' --glob '*.yaml' --glob '*.json' --glob '*.toml' -e 'lvgl' -e 'LVGL' -e 'git@' -e 'github.com' . | sed -n '1,180p'
Repository: TactilityProject/Tactility
Length of output: 41396
Script executed:
printf '%s\n' '--- LVGL indev structure and wait-release implementation ---' nl -ba Libraries/lvgl/src/indev/lv_indev.c | sed -n '40,90p;220,265p;550,585p;780,840p;850,985p;1210,1270p' printf '%s\n' '--- LVGL public contract ---' nl -ba Libraries/lvgl/include/lvgl/indev/lv_indev.h | sed -n '370,405p' printf '%s\n' '--- comparable repository wait-release usage ---' nl -ba Tactility/Source/lvgl/SystemRecents.cpp | sed -n '1,110p' nl -ba Tactility/Source/lvgl/SystemMenu.cpp | sed -n '230,275p' printf '%s\n' '--- gesture source with line numbers ---' nl -ba Tactility/Source/lvgl/SystemGestures.cpp | sed -n '85,205p;215,265p'
Repository: TactilityProject/Tactility
Length of output: 30523
Script executed:
printf '%s\n' '--- pointer processing and press/release dispatch ---' nl -ba Libraries/lvgl/src/indev/lv_indev.c | sed -n '650,815p;1320,1515p' printf '%s\n' '--- reset implementation and all wait-until-release references ---' rg -n -F -- 'wait_until_release' Libraries/lvgl/src/indev/lv_indev.c rg -n -F -- 'lv_indev_reset' Libraries/lvgl/src/indev/lv_indev.c nl -ba Libraries/lvgl/src/indev/lv_indev.c | sed -n '300,385p;680,735p'
Repository: TactilityProject/Tactility
Length of output: 21854
Script executed:
printf '%s\n' '--- pointer press entry and release completion ---' nl -ba Libraries/lvgl/src/indev/lv_indev.c | sed -n '1278,1325p;1487,1545p' printf '%s\n' '--- lv_indev_reset ---' nl -ba Libraries/lvgl/src/indev/lv_indev.c | sed -n '415,465p'
Repository: TactilityProject/Tactility
Length of output: 6734
Script executed:
printf '%s\n' '--- remaining pointer release dispatch ---' nl -ba Libraries/lvgl/src/indev/lv_indev.c | sed -n '1538,1625p' printf '%s\n' '--- reset core ---' rg -n -F -- 'static void indev_reset_core' Libraries/lvgl/src/indev/lv_indev.c nl -ba Libraries/lvgl/src/indev/lv_indev.c | sed -n '1660,1755p'
Repository: TactilityProject/Tactility
Length of output: 8872
Script executed:
printf '%s\n' '--- LVGL combined object/indev event helper ---' rg -n -F -- 'static lv_result_t send_event' Libraries/lvgl/src/indev/lv_indev.c nl -ba Libraries/lvgl/src/indev/lv_indev.c | sed -n '165,220p;1368,1385p' printf '%s\n' '--- gesture callback registration ---' nl -ba Tactility/Source/lvgl/SystemGestures.cpp | sed -n '221,247p'
Repository: TactilityProject/Tactility
Length of output: 4664
Clear the gesture state when Recents suppresses the release callback.
After lv_indev_wait_release(), LVGL clears its active object and skips the LV_EVENT_RELEASED dispatch. The paused hold timer cannot recover SystemGestures state. On the next press, isPressStillDown() returns true, so the press is ignored. Its release then clears the old state without handling the new gesture.
Suggested fix
-
lv_timer_pause(timer); lv_indev_wait_release(state.indev); + clearPress(); systemRecentsShow();
|
Hi! Thank you for the contribution and the work that went into exploring these ideas. There are definitely some interesting directions here that are worth considering. To start with some positives:
The main challenge for me is the scope of the pull request. There are quite a few design decisions to discuss, and reviewing all of them in detail would be difficult to do effectively in a single PR. Rather than go through every screenshot individually, I'll share some broader feedback and explain a few of the constraints that influence the existing designs. General feedback
A few examples of the design constraintsI won't go through every screen individually, but I'd like to explain some of the reasoning behind a couple of the existing design choices. Files app The files app previously used a list-based view similar to the one proposed here. I changed it to a page-based UX because many devices don't have the display performance needed for smooth scrolling. Even on a relatively powerful device like the Tab5, performance can vary significantly depending on orientation. Scrolling is very smooth in portrait mode, which is its default orientation, but in landscape mode the display requires software rotation (with PPA acceleration), making scrolling considerably slower because of the number of pixels that need to be rotated. The proposed redesign is visually appealing, but the list displays only four items at a time. An icon-grid layout, for example, could display three rows of three items, giving us nine items on screen at once. Combined with back/forward paging, this could make navigation faster and more convenient, especially on devices where users primarily navigate using buttons or an encoder. This is a good example of why we need to evaluate UI changes not just by how they look in screenshots, but also by how they perform and work across our supported devices. Looking aheadI'd like to keep the current architecture relatively simple for now, including limiting the amount of code and the testing burden. In the future, I may explore having different window managers for different device classes. For example, the Tab5 could benefit from a desktop-style environment where multiple apps can be visible on screen simultaneously. For now, though, I'd prefer to build toward that incrementally rather than introduce multiple window managers before we have a clear need and design for them. Thanks again for putting these ideas together. There are some useful ideas here, and I'd be happy to discuss individual aspects and consider them in smaller, more focused PRs. Ideally, I'd like to discuss proposed changes first in GitHub Discussions, so we can align on the direction before investing time in implementation. |
Summary by CodeRabbit