-
Notifications
You must be signed in to change notification settings - Fork 31
Symantics #7
Copy link
Copy link
Open
Description
Hey Rick, couple of things.
I like the modified structure, and how your implementing the examples. A couple of points of feedback:
Core.js
- Recommend adding the openScreen method to the Navigator object as a default method for opening controllers - this removes a dependancy at the most basic level. Your "Simple" project example shows how to abstract this to a very simple lib file (navigation.js), but think that core should have this basic ability by default. More complex examples (ViewNavigation for example) appropriately show how to override the Navigator object in core to create custom navigation paradigms.
- We should try and stick to similar naming conventions - openScreen vs open are both referenced in the example apps- for clarity lets just use "open"?
ViewNavigationExample => Navigation Object (lib/navigation.js)
- Setting "this" to "that" - i understand why your doing this, but I find this to be confusing to those that don't have a pretty fairly deep knowledge of javascript, in particular variable scope etc. recommendation is to change this variable name to "_navigation" with a commment about referencing the instance of the object from within internal functions etc.
General Repository Structure
As the repository now stands, I can not simply do an import of the project into Studio and run it - would recommend altering the structure to make that a cleaner process.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
No labels