-
Sub-task
-
Resolution: Fixed
-
Minor
-
2.0
-
MOODLE_20_STABLE
-
MOODLE_20_STABLE
-
dev
Here are some comments based on the dev branch of today, they did not fit in another issue.
mmUtil
- $mmUtil should not need a reference to $mmSite, if $mmUtil::fixPluginFile needs a token, it should always be passed a token.
- Could we rename fixPluginFile to fixPluginFileURL at the same time?
- Could we rename fixPluginFile to fixPluginFileURL at the same time?
- I don't really like $mmUtil.showErrorModal as it will quickly hit issues where we have to pass a lot of parameters to it. If we really want to abstract the modal system, then let's make it a whole service or so. Though I would vote for a guideline and using inline $ionicPopup or whatever else.
- Same goes with loading IMO.
- Why is $mmUtil a provider? Does it need to be used during scaffolding?
- Method $mmUtil.isPhone is redundant is we keep $ionicPlatform.isTablet (app.js)
App.js
- I would leave that almost blank, leaving pretty much only what Ionic has added. Everything that core adds should be in core/main.js
- We should decide whether or not we define the global dependencies in app.js, or in core/main.js. Example: pascalprecht.translate which is likely to be used by third parties too should probably be in app.js.
General
- We should avoid using $mmConfig as a temporary storage to pass data between controllers/services etc... it is probably better to define a basic store where we need it, and request back the value from where it was defined. (Noticed that with mmLogin vs. mmSiteManager)
- Can I suggest the following file naming convention?
- components/courses/controllers/list.js, instead of courseslistctrl.js
- $mmUserDelegate is located in the wrong place
- It seems that we decided to use a services directory rather than lib, see https://tracker.moodle.org/browse/MOBILE-818