search

Famous / famous-angular

Bring structure to your Famo.us apps with the power of AngularJS. Famo.us/Angular integrates seamlessly with existing Angular and Famo.us apps.
https://famo.us/angular
Mozilla Public License 2.0
1.92k stars 275 forks source link

Accept fa-options for Famo.us Engine (#227) #278

Closed pencilcheck closed 9 years ago

pencilcheck commented 9 years ago

fa-options will be too late to set options when createContext is called, for instance, app-mode if not disabled before calling createContext will attach a listener which will disable webapp scrolling.

Haven't thought of a better way to do so, but this is good for now.

zackbrown commented 9 years ago

The tricky piece about this is the fact that each fa-app really encapsulates a Context, not the Engine. Thus, setting Engine options in the Contexts is unclean design at best, and counter-intuitive or unpredictable at worst. With this proposed solution, consider the case where you have two fa-apps in your application and you set fpsCap in the options on one of them. Although you would expect it to affect only one of the fa-app, a developer would be surprised to find that the fpsCap is set on all Contexts because it's an Engine-level config option that affects all existing Contexts .

Rather than this approach of setting Engine options via individual Contexts (you'll note Context itself doesn't have any options; there is no .setOptions on Context, and its constructor argument is a DOM element, not an options hash,) I would recommend using a run block to configure the Engine at app bootstrap time. Something like:

myModule.run(function($famous){
  var Engine = $famous['famous/core/Engine'];
  var engineOptions = {
    appMode: false
  };
  Engine.setOptions(engineOptions);
});

We really appreciate the time and effort you took to make this contribution, but for the outlined reasons we will not be merging this PR at this time.

pencilcheck commented 9 years ago

I see. If that's the case, that makes more sense to do that in run block. Is it possible to update the README with this information? Because I believe a lot more users are using famous as an extra animation framework with another UI framework (e.g. hybrid mobile development like ionic). Disable app mode would be desirable.