Moodle
  1. Moodle
  2. MDL-30175

Add support into modinfo/cm_info to external URLs defining activity icons

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.2
    • Fix Version/s: 2.2
    • Component/s: Course, General, Libraries
    • Labels:
    • Testing Instructions:
      Hide

      IMPORTANT: Code modifications are needed to test this, because there isn't any core-activity yet using external activity icons.

      0) Install/upgrade/use moodle from integration.git
      1) Go to course with some forums and some other activities.
      2) TEST: Verify that icons are shown properly in the course page.
      3) Edit mod/forum/lib.php and create this function

      function forum_get_coursemodule_info($coursemodule) {
       
          $info = new cached_cm_info();
          $info->iconurl = new moodle_url('http://www.euroblind.org/skin/basic/design/braille/i/' . ($coursemodule->id % 10) . '.gif');
          return $info;
      }

      (that will enable the forum module to use the new feature, picking some "mod-based" images from outside)

      4) Reload the course page, you should not see any modification (because modinfo is cached).
      5) Edit any forum, and click "save and go to course page".
      6) TEST: Voilà, you should see the forum icons changed to some braille-icons.
      7) TEST: The rest of activities continue showing their default icons.
      8) Delete the code introduced by 3)
      9) Reload the course page, you should continue viewing the braille icons (because modinfo is cached).
      10) Edit any forum, and click "save and go to course page".
      11) TEST: Back to normal, forums showing their default icons again.

      That is, ciao

      Show
      IMPORTANT: Code modifications are needed to test this, because there isn't any core-activity yet using external activity icons. 0) Install/upgrade/use moodle from integration.git 1) Go to course with some forums and some other activities. 2) TEST: Verify that icons are shown properly in the course page. 3) Edit mod/forum/lib.php and create this function function forum_get_coursemodule_info($coursemodule) {   $info = new cached_cm_info(); $info->iconurl = new moodle_url('http://www.euroblind.org/skin/basic/design/braille/i/' . ($coursemodule->id % 10) . '.gif'); return $info; } (that will enable the forum module to use the new feature, picking some "mod-based" images from outside) 4) Reload the course page, you should not see any modification (because modinfo is cached). 5) Edit any forum, and click "save and go to course page". 6) TEST: Voilà, you should see the forum icons changed to some braille-icons. 7) TEST: The rest of activities continue showing their default icons. 8) Delete the code introduced by 3) 9) Reload the course page, you should continue viewing the braille icons (because modinfo is cached). 10) Edit any forum, and click "save and go to course page". 11) TEST: Back to normal, forums showing their default icons again. That is, ciao
    • Affected Branches:
      MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE
    • Pull from Repository:
    • Pull Master Branch:

      Description

      This is a new feature introduced to fulfill the need of some modules (usually calling to external stuff) to use activity icons stored out from moodle codebase.

      Its primary target is to be used in the new ims-lti module though could be extended, once available to other new modules (current core ones should not need it at all).

      So this can be considered related with MDL-20534 and aimed to solve the points A6/B3 @ http://docs.moodle.org/dev/IMS-LTI_consumer_module_review_2011-11

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            Eloy Lafuente (stronk7) added a comment -

            From MDL-20534:

            Sam:

            Eloy's patch looks 100% right to me except:

            1) I would probably define it as a moodle_url not a string (this is only new code so no need for legacy report)? Or, is there problem with serialising moodle_url into the modinfo cache?

            2) Trivial but there's a @var string; <-- unnecessary semicolon

            Looking to them right now, I always get confused with moodle_urls, grrr, thanks!

            Show
            Eloy Lafuente (stronk7) added a comment - From MDL-20534 : Sam: Eloy's patch looks 100% right to me except: 1) I would probably define it as a moodle_url not a string (this is only new code so no need for legacy report)? Or, is there problem with serialising moodle_url into the modinfo cache? 2) Trivial but there's a @var string; <-- unnecessary semicolon Looking to them right now, I always get confused with moodle_urls, grrr, thanks!
            Hide
            Eloy Lafuente (stronk7) added a comment -

            Changes above implemented.

            Note that it's not enforced at all so it will continue accepting strings via cached_cm, only set_icon_url() enforces it. I think it's ok for now, we can add coding_exception later, but it will be detected @ distance, so... not 100% useful.

            Sending to integration...

            Show
            Eloy Lafuente (stronk7) added a comment - Changes above implemented. Note that it's not enforced at all so it will continue accepting strings via cached_cm, only set_icon_url() enforces it. I think it's ok for now, we can add coding_exception later, but it will be detected @ distance, so... not 100% useful. Sending to integration...
            Hide
            Sam Hemelryk added a comment -

            Thanks Eloy - changes looked spot on.
            This has been integrated now

            Show
            Sam Hemelryk added a comment - Thanks Eloy - changes looked spot on. This has been integrated now
            Hide
            Rajesh Taneja added a comment -

            Works as mentioned
            Thanks for adding this feature Eloy.

            Show
            Rajesh Taneja added a comment - Works as mentioned Thanks for adding this feature Eloy.
            Hide
            Eloy Lafuente (stronk7) added a comment -

            Thanks!

            Offtopic: SamH you were really lucky with this because it seems that navigation goes in a a harcoded way to icon/iconcomponent, so this new external iconurls does not cause MDL-26786 to happen. In any case I think that, sooner than later, it will be required to support those icons in navigation, so the fix for that issue should land 100% sure.

            Show
            Eloy Lafuente (stronk7) added a comment - Thanks! Offtopic: SamH you were really lucky with this because it seems that navigation goes in a a harcoded way to icon/iconcomponent, so this new external iconurls does not cause MDL-26786 to happen. In any case I think that, sooner than later, it will be required to support those icons in navigation, so the fix for that issue should land 100% sure.
            Hide
            Eloy Lafuente (stronk7) added a comment -

            Yes, you got this finally upstream, just in time for Moodle 2.2beta. Congrats and thanks!

            Ciao

            Show
            Eloy Lafuente (stronk7) added a comment - Yes, you got this finally upstream, just in time for Moodle 2.2beta. Congrats and thanks! Ciao

              People

              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: