-
-
Notifications
You must be signed in to change notification settings - Fork 47
Make edit mode global across menu items #395
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughThe edit-mode state was centralized in MenuItem by adding a static bool Sequence Diagram(s)sequenceDiagram
participant User
participant Menu
participant MenuItem
participant Renderer
User->>Menu: Press ENTER
Menu->>MenuItem: beginEdit()
Note over MenuItem: MenuItem._isEditing = true
User->>Menu: Navigate / Modify while editing
Menu->>MenuItem: isEditing()
MenuItem-->>Menu: true
Menu->>Renderer: draw() -- queries MenuItem::isEditing()
User->>Menu: Press BACK
Menu->>MenuItem: endEdit()
Note over MenuItem: MenuItem._isEditing = false
Menu->>Renderer: draw() -- renderer no longer stores edit state
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
|
Memory usage change @ a9b3798
Click for full report table
Click for full report CSV |
|
Memory usage change @ 33accd9
Click for full report table
Click for full report CSV |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
.scripts/prepare_workflow.py(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: run-ci (examples/ItemBack/ItemBack.ino)
- GitHub Check: run-ci (examples/List/List.ino)
- GitHub Check: run-ci (examples/UseByRef/UseByRef.ino)
- GitHub Check: run-ci (examples/MenuTimeout/MenuTimeout.ino)
- GitHub Check: run-ci (examples/IntFloatValues/IntFloatValues.ino)
- GitHub Check: run-ci (examples/ViewShifting/ViewShifting.ino)
- GitHub Check: run-ci (examples/CharsetInput/CharsetInput.ino)
- GitHub Check: run-ci (examples/Widgets/Widgets.ino)
- GitHub Check: run-ci (examples/SubMenu/SubMenu.ino)
- GitHub Check: run-ci (examples/Basic/Basic.ino)
- GitHub Check: run-ci (examples/Callbacks/Callbacks.ino)
- GitHub Check: run-ci (examples/ButtonAdapter/ButtonAdapter.ino)
- GitHub Check: compile (esp32:esp32:esp32, esp32, esp32-esp32-esp32)
🔇 Additional comments (1)
.scripts/prepare_workflow.py (1)
21-28: Review comment is based on incorrect assumptions about the code.The review comment claims a 10x timing reduction was introduced by removing
hold_time = press_holding_time * 10, but the actual code contains no such variable or scaling factor. The current implementation consistently usespress_holding_timedirectly in both the template (line 27) and the wait calculation (line 66):
button_down_template:- delay: {press_holding_time}msdown_wait = press_holding_time + serial_wait_timeThe templates and calculations are already aligned and consistent. There is no evidence of the timing change described in the review comment.
Likely an incorrect or invalid review comment.
|
Memory usage change @ c60b69d
Click for full report table
Click for full report CSV |
|
Memory usage change @ bb2ecc4
Click for full report table
Click for full report CSV |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Memory usage change @ 263bca8
Click for full report table
Click for full report CSV |
…_wait and up_wait calculations
|
Memory usage change @ 0ab6b3b
Click for full report table
Click for full report CSV |
Summary
MenuItemMenuRendererTesting
g++ -std=c++0x test/utils.cpp test/LcdMenu.cpp test/ItemValue.cpp test/UnselectableItem.cpp test/MenuScreenTest.cpp -I src -I test -o .arduino_ci/tests.out(fails: Arduino headers missing)pio run(fails: undefined reference tosetup/loop)wokwi-cli --version(fails: command not found)https://chatgpt.com/codex/tasks/task_e_685c369853888332875ee095da3f4ba3
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Tests
Chores