jonblack / arduino-menusystem

Arduino library for implementing a menu system
MIT License
194 stars 85 forks source link

Cannot set root menu name #69

Open gnikolopoulos opened 7 years ago

gnikolopoulos commented 7 years ago

I am trying to to create a menu where the renderer will also display the parent item name on the current item. For example, think of this structure:

Main Menu
- Item 1
-- Item A
-- Item B
- Item 2
- Item 3

So, when browsing the menu, you will get something like this:

Line 1: Main Menu
Line 2: Item1

Or this:

Line 1: Item 1
Line 2: Item A

This is my current (and early) implementation of a renderer:

class Renderer : public MenuComponentRenderer {
public:
    void render(Menu const& menu) const {
        oled.clearDisplay();
        menu.get_current_component()->render(*this);
    }

    void render_menu_item(MenuItem const& menu_item) const {
        oled.setTextSize(1);
        oled.setTextColor(WHITE);
        oled.setCursor(0,0);
        oled.println("Menu Item");
        oled.setTextSize(2);
        oled.setTextColor(WHITE);
        oled.setCursor(0,17);
        oled.println(menu_item.get_name());
        oled.display();
    }

    void render_back_menu_item(BackMenuItem const& menu_item) const {}

    void render_numeric_menu_item(NumericMenuItem const& menu_item) const {}

    void render_menu(Menu const& menu) const {
        oled.setTextSize(1);
        oled.setTextColor(WHITE);
        oled.setCursor(0,0);
        oled.println("Render menu");
        oled.setTextSize(2);
        oled.setTextColor(WHITE);
        oled.setCursor(0,17);
        oled.print(menu.get_name());
        oled.display();
    }
};
Renderer menu_render;

I can see there are methods like get_parent(), but I am not sure of how to implement them. Can you please point me to the right direction? I am quite new to arduino programming...

jonblack commented 7 years ago

Given your menu structure above, when the current menu is the root "Main Menu", the first line displays "Main Menu" and the second line allows you to scroll through the components in that menu. Is that what you mean?

When you call MenuSystem::display it will call Renderer::render with the current menu (in this case "Main Menu"). If you first display the menu's name in that render method before rendering it's current component, that should do the trick:

class Renderer : public MenuComponentRenderer {
public:
    void render(Menu const& menu) const {
        oled.clearDisplay();
        // code to output menu.get_name() to line 1 here
        menu.get_current_component()->render(*this); // <-- this will trigger rendering for the current item
    }

    // Code omitted. Each menu component render function should output to line 2
};

I hope that makes sense. If you have any questions (or it doesn't work) please let me know.

gnikolopoulos commented 7 years ago

You are right... the answer was in front of me... but what happens with the root menu? I am using a declaration like below for the menu, and the menu.get_name() comes up empty when I navigate the level 0 (Item 1,2, etc) Here is a sample of my menu definitions:

MenuSystem ms(menu_render);
Menu mm("Main Menu");
MenuItem mi_i1("Item 1", &on_menu_item1);
MenuItem mi_i2("Item 2", &on_menu_item2);
Menu mi_i3("Item 3");
MenuItem mi_i3_a("Item A", &on_menu_i3a);
MenuItem mi_i3_b("Item B", &on_menu_i3b);
jonblack commented 7 years ago

The MenuSystem class creates the top-level root menu for you, and it's name is an empty string, so internally your menu structure above looks like the following:

* "" // this is created automatically
  * main menu
    * Item 1
    * Item 2
    * Item 3
      * Item A
      * Item B

This assumes you're using MenuSystem::get_root_menu().add_xxx to connect your MenuComponent instances to MenuSystem.

In previous version of this library the client would create the root manually and pass it in the MenuSystem constructor. I changed it to create it automatically because it was never displayed (or so I thought) and therefore felt awkward to create.

So that's the why. The next thing is how do we solve your problem. Great question! :smile:

I still think it's a good idea for MenuSystem to create the root Menu. The problem is that it assumes it won't be displayed and sets its name to an empty string. I think the best solution is to add an optional char* root_name parameter to the MenuSystem constructor for setting the name of the root Menu.

I'm open to other suggestions if you have them.

gnikolopoulos commented 7 years ago

Well, one way could be to see add back the ability to set the root menu, along with a name, like v2.1. If the user has defined one, use it. Otherwise, create the root menu automatically, like in v3.0. For now, I just modified the render method to check if the menu name is empty and display a pre-defined string if it is. Not too hacky, and is the simplest solution I could find, without modifying the library...

jonblack commented 7 years ago

I prefer the current implementation where the root is created for you. I'll add an extra, optional parameter for setting the name.

pserotta commented 7 years ago

Hi, I am facing the same issue, was an update posted with the option to setting the root name? Thank you!