Details

    • Type: Sub-task
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0
    • Fix Version/s: 2.0
    • Component/s: Ionic
    • Labels:

      Description

      Here are some comments based on the dev branch of today, they did not fit in another issue.

      mmUtil

      1. $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?
      2. 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.
      3. Why is $mmUtil a provider? Does it need to be used during scaffolding?
      4. Method $mmUtil.isPhone is redundant is we keep $ionicPlatform.isTablet (app.js)

      App.js

      1. I would leave that almost blank, leaving pretty much only what Ionic has added. Everything that core adds should be in core/main.js
      2. 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

      1. 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)
      2. Can I suggest the following file naming convention?
        • components/courses/controllers/list.js, instead of courseslistctrl.js
      3. $mmUserDelegate is located in the wrong place
      4. It seems that we decided to use a services directory rather than lib, see https://tracker.moodle.org/browse/MOBILE-818

        Attachments

          Activity

            People

            Assignee:
            dpalou Dani Palou
            Reporter:
            fred Frédéric Massart
            Peer reviewer:
            Frédéric Massart Frédéric Massart
            Integrator:
            Juan Leyva Juan Leyva
            Participants:
            Component watchers:
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

              Dates

              Created:
              Updated:
              Resolved:
              Fix Release Date:
              31/Jul/15