Skip to content

Refactored some of the system menus to the new DRY method#8997

Closed
thebentern wants to merge 231 commits intodevelopfrom
refactor-menus
Closed

Refactored some of the system menus to the new DRY method#8997
thebentern wants to merge 231 commits intodevelopfrom
refactor-menus

Conversation

@thebentern
Copy link
Contributor

🙏 Thank you for sending in a pull request, here's some tips to get started!

❌ (Please delete all these tips and replace them with your text) ❌

  • Before starting on some new big chunk of code, it it is optional but highly recommended to open an issue first
    to say "Hey, I think this idea X should be implemented and I'm starting work on it. My general plan is Y, any feedback
    is appreciated." This will allow other devs to potentially save you time by not accidentially duplicating work etc...
  • Please do not check in files that don't have real changes
  • Please do not reformat lines that you didn't have to change the code on
  • We recommend using the Visual Studio Code editor along with the 'Trunk Check' extension (In beta for windows, WSL2 for the linux version),
    because it automatically follows our indentation rules and its auto reformatting will not cause spurious changes to lines.
  • If your PR fixes a bug, mention "fixes #bugnum" somewhere in your pull request description.
  • If your other co-developers have comments on your PR please tweak as needed.
  • Please also enable "Allow edits by maintainers".
  • Please do not submit untested code.
  • If you do not have the affected hardware to test your code changes adequately against regressions, please indicate this, so that contributors and commnunity members can help test your changes.
  • If your PR gets accepted you can request a "Contributor" role in the Meshtastic Discord

🤝 Attestations

  • I have tested that my proposed changes behave as described.
  • I have tested that my proposed changes do not cause any obvious regressions on the following devices:
    • Heltec (Lora32) V3
    • LilyGo T-Deck
    • LilyGo T-Beam
    • RAK WisBlock 4631
    • Seeed Studio T-1000E tracker card
    • Other (please specify below)

HarukiToreda and others added 30 commits September 21, 2025 17:40
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors multiple system menu implementations in the MenuHandler to use a new DRY (Don't Repeat Yourself) pattern via the createStaticBannerOptions template function. The refactoring replaces manual array management and callback logic with a type-safe, template-based approach using MenuOption<T> structures.

Key changes:

  • Introduces ScreenColor struct and several type aliases (LoraRegionOption, TimezoneOption, CompassOption, etc.) to support strongly-typed menu options
  • Refactors 9 menu functions to use the new pattern: LoraRegionPicker, ClockFacePicker, TZPicker, positionBaseMenu, nodeNameLengthMenu, compassNorthMenu, GPSToggleMenu, GPSFormatMenu, and TFTColorPickerMenu
  • Fixes string literal split issues in the original code (e.g., "MY_" "919" → "MY_919")

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/graphics/draw/MenuHandler.h Adds ScreenColor struct to encapsulate RGB color values with a useVariant flag, and introduces type aliases for various menu option types to improve type safety
src/graphics/draw/MenuHandler.cpp Refactors 9 menu functions from manual array/callback pattern to the new DRY createStaticBannerOptions pattern, improving maintainability and consistency while fixing string literal formatting issues

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@jp-bennett jp-bennett added the cleanup Code cleanup or refactor label Dec 16, 2025
Base automatically changed from multi-message-Storage to develop December 24, 2025 22:13
@Xaositek
Copy link
Contributor

Xaositek commented Jan 2, 2026

Closing this for the moment

@Xaositek Xaositek closed this Jan 2, 2026
@Xaositek Xaositek deleted the refactor-menus branch January 2, 2026 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cleanup Code cleanup or refactor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants