Fix + Refactor : crash issue of Action Test in Tests App

[Blackhawk95]

• Fixed the crash happening on pressing any button after action menu loads
• Changed from "MenuLayer menu_layer" in ActionMenu to "MenuLayer *menu_layer"
• Changes associated with previous change
• Moved struct declaration to action_menu.h from action_menu.c
• Removed const from struct ( I felt it is bad to give const in struct )
• Created ctor, dtor, create and destroy functions similar to menu_layer
• Cast to ClickHandler for button callbacks
• Added "void context" like other _layer ui elements in ActionMenu{ } instead of casting context to ActionMenu
• KNOWN ISSUE - Side changes colour during animation/transition Doesn't seem to be related to changes I made

[jwise]

First off, thanks for taking a look at this! A few things -- I understand why you made some of these changes, but I think they were based on incomplete information :) So I'll try to fill you in.

• It's generally a reasonable idea to move public structs off to header files. The reason why those lived in action_menu.c, rather than in the header file, was because they don't present a public interface -- nothing outside of action_menu.c should be touching those (those are all internal 'bookkeeping' for an action menu). So that's why those were in action_menu.c -- and, probably, why they ought to stay there.
• The trick where action_menu_level_create did all its work in one allocation was a clever way to save some memory (actually, in two ways). The key insight there is that the length of items never changes, and so we can allocate memory at the same time for both the items and the ActionMenuLevel itself -- and we used the special syntax ActionMenuItems items[] (only valid at the end of a struct...) to tell the compiler that the items were always placed in memory right after the ActionMenuLevel itself.
• The general idea behind the context scheme is that it allows us to have multiple ActionMenus on the system, with only one set of functions to refer to them. (Maybe an ActionMenu opens up another window, which could in turn open up its own ActionMenu on top?) To allow us to have multiple ActionMenus, we make the caller be responsible for holding on to the ActionMenu pointer, rather than having global state (the static ActionMenu *action_menu that this change introduces). The problem that this introduces is: when the ClickRecognizer code calls us back, how do we know which ActionMenu it was talking about? The trick to this is this context scheme: we ask the click config mechanism to "remind" us of which ActionMenu we were talking about by asking it to hand us that pointer back as the context.
• The final piece of the puzzle where we cast Windows to ActionMenus is that the Window is the first element of an ActionMenu. We know that if we're getting called with an action_menu function that hands us a Window, it has to be the Window that is part of the ActionMenu -- so we can cast that window directly to an ActionMenu and get access to the rest of the structure. (This is a lot like how we make ActionMenuItems part of ActionMenuLevel.)
• One note about const struct elements: this is actually a really confusing part of C. In this case, the const struct element doesn't refer to the pointer in the struct, but to what the pointer points to. By way of example:

  
  struct s {
  const int *i;
  }

void do_something(struct s s) { int local_i = 4; s->i = &local_i; / this is allowed / (s->i) = 5; / this is not allowed; i is a const pointer / }

This is all really tricky stuff, so don't feel too bad!  It's exciting to see you dive into it.  I'd encourage you to take another crack at it, maybe starting again from the original implementation, to see if there's a smaller set of changes you can make to fix it.

Thanks again!