adobe/brackets — pull request #927 by tvoliter (2012-05-23 16:40:32+00:00)
Title: Menu, MenuItem, key bindings, and Command API (review only)

This is round 2 of the Menu and key binding API review. The previous review can be found here: https://github.com/adobe/brackets/pull/91. I submitted a new pull request to reduce comment clutter and since much has changed with the design.

Please review **API only** and not the implementation. Current implementation is not well tested and is mostly prototyped to inform the API design

**Change Summary**
* Insertion position for Menus and MenuItems is now uniformly specified in the API via two parameters: “relativeID” and a “position” constant. Menu.js defines an “enum” for the position constants. Omitting both parameters means insert after the last existing item.

* createMenu() and createMenuItem have been renamed to use “add” instead of “create”.

* AddMenu() and AddMenuItem() now return the object that was created. Earlier they supported JQuery chaining by returning the object they were being added to.

* Added map objects to Menus.js store all Menu and MenuItems created. Added lookup functions.

* Improved documentation for createMenuItemsFromJSON() and also changed function to take an array of MenuItem descriptions.

* Most of the state that was in MenuItem (enabled, checked, etc) has been moved to the Command object, so MenuItems now function as a view of Commands. Question: where should the “name” property live> In the Command or MenuItem?

* Added Command object to CommandManager. Commands have an id, executing function, enabled state, checked state, and an isEnabled() callbackfunction.

* Implemented most of the Command, MenuItem, and Menu API to vet how Commands and MenuItems would interact. Brackets menus are now created dynamically.

* Implementation hooks up to Rays new API for adding new key bindings after app launch

* Simplified MenuItem creation API by removing “attributes” and many event callbacks since Commands own this now. API still allows clients to create MenuItems and specify key bindings, but the docs make it clear that the bindings are mapped to the underlying Command, not the MenuItem.

* added APIs for removing Menus and MenuItems

* removed all event handlers (click, rollout, etc) from MenuItem. MenuItems just execute their associated Command object. Enabled/checked state is pushed to the related Command and the MenuItem reflects it.

**Still working on**
* Implementation. A lot has been implemented, but the API needs to settle first for the implementation is polished and tested
* “name” still lives in MenuItems not Commands. Need to think more about this.
* figuring out how MenuItems reflect the enabled/checked state of the Command when they are shown
* Some Brackets APIs pass command ID’s some now pass Command objects. Need to figure out if we need to be consistent or allow both