Moodle
  1. Moodle
  2. MDL-37749

MyMobile theme contains Collapsed Topics contributed plugin code.

    Details

    • Type: Task Task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3.4, 2.4.1
    • Fix Version/s: 2.3.5, 2.4.2, 2.5
    • Component/s: Themes
    • Labels:
    • Testing Instructions:
      Hide
      1. With iPod Touch or other device / simulator confirm that the pre-login screen is as screen shot 'MDL-37749_before_change_before_login.PNG'.
      2. Ensure that the test course (From 'backup-moodle2-course-2-ct-20130205-0959-nu.mbz' or own similar) set to the 'Topics' format.
      3. With iPod Touch or other device / simulator, login and go to the test course, and confirm that the screen is as screen shot 'MDL-37749_before_change_topics.PNG'.
      4. Change the course format of the test course to 'Collapsed Topics' via the 'Edit settings' option of the course.
      5. With iPod Touch or other device / simulator, refresh the test course, and confirm that the screen is as screen shot 'MDL-37749_before_change_topcoll.PNG'.
      6. Logout of Moodle, perform the code changes and do a 'Purge all caches'.
      7. With iPod Touch or other device / simulator confirm that the pre-login screen is as screen shot 'MDL-37749_after_change_before_login.PNG'.
      8. Change the course format of the test course to 'Topics' via the 'Edit settings' option of the course.
      9. With iPod Touch or other device / simulator, login and go to the test course, and confirm that the screen is as screen shot 'MDL-37749_after_change_topics.PNG'.
      10. Change the course format of the test course to 'Collapsed Topics' via the 'Edit settings' option of the course.
      11. With iPod Touch or other device / simulator, refresh the test course, and confirm that the screen is as screen shot 'MDL-37749_after_change_topcoll.PNG'.
      Show
      With iPod Touch or other device / simulator confirm that the pre-login screen is as screen shot ' MDL-37749 _before_change_before_login.PNG' . Ensure that the test course (From 'backup-moodle2-course-2-ct-20130205-0959-nu.mbz' or own similar) set to the 'Topics' format. With iPod Touch or other device / simulator, login and go to the test course, and confirm that the screen is as screen shot ' MDL-37749 _before_change_topics.PNG' . Change the course format of the test course to 'Collapsed Topics' via the 'Edit settings' option of the course. With iPod Touch or other device / simulator, refresh the test course, and confirm that the screen is as screen shot ' MDL-37749 _before_change_topcoll.PNG' . Logout of Moodle, perform the code changes and do a 'Purge all caches'. With iPod Touch or other device / simulator confirm that the pre-login screen is as screen shot ' MDL-37749 _after_change_before_login.PNG'. Change the course format of the test course to 'Topics' via the 'Edit settings' option of the course. With iPod Touch or other device / simulator, login and go to the test course, and confirm that the screen is as screen shot ' MDL-37749 _after_change_topics.PNG' . Change the course format of the test course to 'Collapsed Topics' via the 'Edit settings' option of the course. With iPod Touch or other device / simulator, refresh the test course, and confirm that the screen is as screen shot ' MDL-37749 _after_change_topcoll.PNG' .
    • Affected Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE
    • Pull 2.4 Branch:
      MDL-37749_M24
    • Rank:
      47475

      Description

      The MyMobile theme contains the following code in 'custom.js':

      //collapsed topic only stuff
      $('div#page-course-view-topcollPAGE').live('pagebeforecreate',function(event, ui){
          $('#page-course-view-topcollPAGE ul.section').attr("data-role", "none");
          $('.section li img').removeClass("ui-li-icon");
          $.getScript('../course/format/topcoll/module.js');
          $('#page-course-view-topcollPAGE tr.cps a').attr("data-role", "button").attr("data-icon", "arrow-r");
          $('#page-course-view-topcollPAGE #thetopics').attr("data-role", "controlgroup");
          $('#page-course-view-topcollPAGE td.cps_centre').each(function(index) {
              var cpsc = $(this).text().replace('<br>','').replace(')','');
              $(this).prev('td').find('a').append('<span class="ui-li-count ui-btn-up-a ui-btn-corner-all">' + cpsc + '</span>');
          });
      });
      

      In 'core.css' the following selectors need to be removed:

      /*collapsed topic format*/
      #thetopics {
          table-layout: inherit !important;
          width: 100%;
          display: block !important;
      }
      #page-course-view-topcollPAGE .section td.content, col.content {
          text-align: left;
          width: 100% !important;
          overflow: hidden;
      }
      #page-course-view-topcollPAGE .section td.content {
          padding: .5em;
      }
      #page-course-view-topcollPAGE td.left.side, #page-course-view-topcollPAGE td.right.side {
          display: none;
      }
      #page-course-view-topcollPAGE tr.cps td a {
          background: none !important;
          color: inherit !important;
          padding: 7px 0 7px 0px !important;
      }
      tr.cps td span {
          font-style: inherit !important;
          font-size: 1.12em !important;
      }
      #page-course-view-topcollPAGE .section.separator, td.cps_centre {
          display: none;
      }
      tr.cps {
          background-color: inherit !important;
          color: inherit !important;
      }
      .opencps .ui-btn-inner .ui-icon-arrow-r {
          background-position: -216px 50%;
      }
      tr.cps td span.ui-li-count {
          font-size: .6em !important;
          right: 12px;
      }
      #page-course-view-topcollPAGE .cps .ui-btn-inner {
          white-space: normal;
      }
      

      It is out of date as Collapsed Topics has moved on and as discussed on MDL-33115, core really should not have code that copes with contributed plugins. I am now in a position to adapt my code both with jQuery and styles to support MyMobile but this code is redundant.

      I'm raising this as a separate issue to MDL-33115 because I believe that MDL-33115 is a container for a whole range of issues that are no longer relevant and this small change would get lost within it. A big thank you to Mary Evans for assisting me with it.

      1. iOS Simulator Screen shot 5 Feb 2013 09.48.34.png
        47 kB
      2. iOS Simulator Screen shot 5 Feb 2013 09.49.02.png
        34 kB
      3. MDL-37749_after_change_before_login.PNG
        88 kB
      4. MDL-37749_after_change_topcoll.PNG
        107 kB
      5. MDL-37749_after_change_topics.PNG
        90 kB
      6. MDL-37749_before_change_before_login.PNG
        89 kB
      7. MDL-37749_before_change_topcoll.PNG
        109 kB
      8. MDL-37749_before_change_topics.PNG
        90 kB
      9. missing_bottom_line.png
        66 kB

        Issue Links

          Activity

          Hide
          Mary Evans added a comment -

          Thanks Gareth, I'll fix this ready for next pull which, all being well will be next weekend.

          Show
          Mary Evans added a comment - Thanks Gareth, I'll fix this ready for next pull which, all being well will be next weekend.
          Hide
          Gareth J Barnard added a comment -

          Dear Mary,

          Thank you so much for your rapid deployment of the change - I am in 'I'm not worthy' mode . If possible, Moodle 2.2 and below does not need to be changed as I was still using tables then and the code is needed .

          This will really help me make progress especially with the removal of 'opencps' css style.

          Thanks,

          Gareth

          Show
          Gareth J Barnard added a comment - Dear Mary, Thank you so much for your rapid deployment of the change - I am in 'I'm not worthy' mode . If possible, Moodle 2.2 and below does not need to be changed as I was still using tables then and the code is needed . This will really help me make progress especially with the removal of 'opencps' css style. Thanks, Gareth
          Hide
          Eloy Lafuente (stronk7) added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Hide
          Mary Evans added a comment -

          ALL BRANCHES REBASED

          Show
          Mary Evans added a comment - ALL BRANCHES REBASED
          Hide
          Dan Poltawski added a comment -

          Attaching screenshots to show before/after these changes (the same can be seen just using a standard browser)

          Show
          Dan Poltawski added a comment - Attaching screenshots to show before/after these changes (the same can be seen just using a standard browser)
          Hide
          Dan Poltawski added a comment -

          Hi Mary,

          Sorry, but i've just applied this to master and it appears to be completely breaking the theme so i'm reopening this (See the screenshots i've just attached).

          Please can I request that you do the minimum changes possible on the stable branches (it looks like a lot of the changes are related to line length, please can you only change what is necessary to fix this bug for the stable branches, we)

          Also need testing instructions for this change (both with and without the collapsed topics format).

          thanks,
          Dan

          Show
          Dan Poltawski added a comment - Hi Mary, Sorry, but i've just applied this to master and it appears to be completely breaking the theme so i'm reopening this (See the screenshots i've just attached). Please can I request that you do the minimum changes possible on the stable branches (it looks like a lot of the changes are related to line length, please can you only change what is necessary to fix this bug for the stable branches, we) Also need testing instructions for this change (both with and without the collapsed topics format). thanks, Dan
          Hide
          CiBoT added a comment -

          Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

          Show
          CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
          Hide
          Gareth J Barnard added a comment -

          Hi all,

          It does look like it's the line length causing the issue. I will provide testing instructions in the comments (as I cant edit the tracker).

          Information on the before and after can be seen on https://moodle.org/mod/forum/discuss.php?d=221403 - however, I will post additional screen shots here.

          When testing, it might be advisable to use the Moodle 2.4 branch as my master branch has a tendency to break with all the good work being carried out by Marina Glancy, but at the moment it works, so you can get a version for master from https://github.com/gjb2048/moodle-format_topcoll - or if that fails, use the latest 2.4 version from https://moodle.org/plugins/pluginversions.php?plugin=format_topcoll.

          However can I annotate what the changes are and why you will see noticeable difference in most cases but logic dictates that the code is redundant and inefficient:

          In custom.js (// comments at the end):

          //collapsed topic only stuff
          $('div#page-course-view-topcollPAGE').live('pagebeforecreate',function(event, ui){  // Overall selector that applies to Collapsed Topics only courses / pages.
              $('#page-course-view-topcollPAGE ul.section').attr("data-role", "none");  // Causes issue as seen on https://moodle.org/mod/forum/discuss.php?d=221403
              $('.section li img').removeClass("ui-li-icon"); // I'm told by Michele Turre that this causes an issue.
              $.getScript('../course/format/topcoll/module.js');  // Does nothing.
              $('#page-course-view-topcollPAGE tr.cps a').attr("data-role", "button").attr("data-icon", "arrow-r");  // Selector died with introduction of Moodle 2.3 version of Collapsed Topics so redundant.
              $('#page-course-view-topcollPAGE #thetopics').attr("data-role", "controlgroup"); // Selector died with introduction of Moodle 2.3 version of Collapsed Topics so redundant.
              $('#page-course-view-topcollPAGE td.cps_centre').each(function(index) { // Selector died with introduction of Moodle 2.3 version of Collapsed Topics so redundant.
                  var cpsc = $(this).text().replace('<br>','').replace(')','');
                  $(this).prev('td').find('a').append('<span class="ui-li-count ui-btn-up-a ui-btn-corner-all">' + cpsc + '</span>'); // Selector died with introduction of Moodle 2.3 version of Collapsed Topics so redundant.
              });
          });
          

          In 'core.css' (again with /* */ comments at the end of the line):

          /*collapsed topic format*/
          #thetopics { /* Selector died with introduction of Moodle 2.3 version of Collapsed Topics so redundant. */
              table-layout: inherit !important;
              width: 100%;
              display: block !important;
          }
          #page-course-view-topcollPAGE .section td.content, col.content { /* Selector died with introduction of Moodle 2.3 version of Collapsed Topics so redundant. */
              text-align: left;
              width: 100% !important;
              overflow: hidden;
          }
          #page-course-view-topcollPAGE .section td.content { /* Selector died with introduction of Moodle 2.3 version of Collapsed Topics so redundant. */
              padding: .5em;
          }
          #page-course-view-topcollPAGE td.left.side, #page-course-view-topcollPAGE td.right.side { /* Selector died with introduction of Moodle 2.3 version of Collapsed Topics so redundant. */
              display: none;
          }
          #page-course-view-topcollPAGE tr.cps td a { /* Selector died with introduction of Moodle 2.3 version of Collapsed Topics so redundant. */
              background: none !important;
              color: inherit !important;
              padding: 7px 0 7px 0px !important;
          }
          tr.cps td span { /* Selector died with introduction of Moodle 2.3 version of Collapsed Topics so redundant. */
              font-style: inherit !important;
              font-size: 1.12em !important;
          }
          #page-course-view-topcollPAGE .section.separator, td.cps_centre { /* Selector died with introduction of Moodle 2.3 version of Collapsed Topics so redundant. */
              display: none;
          }
          tr.cps { /* Selector died with introduction of Moodle 2.3 version of Collapsed Topics so redundant. */
              background-color: inherit !important;
              color: inherit !important;
          }
          .opencps .ui-btn-inner .ui-icon-arrow-r { /* Has no effect as toggles not currently rendered by jQueryMobile - this is my issue to fix in CONTRIB-3655. */
              background-position: -216px 50%;
          }
          tr.cps td span.ui-li-count { /* Selector died with introduction of Moodle 2.3 version of Collapsed Topics so redundant. */
              font-size: .6em !important;
              right: 12px;
          }
          #page-course-view-topcollPAGE .cps .ui-btn-inner { /* Has no effect as toggles not currently rendered by jQueryMobile - this is my issue to fix in CONTRIB-3655. */
              white-space: normal;
          }
          

          Screen shots and testing instructions to follow.

          Cheers,

          Gareth

          Show
          Gareth J Barnard added a comment - Hi all, It does look like it's the line length causing the issue. I will provide testing instructions in the comments (as I cant edit the tracker). Information on the before and after can be seen on https://moodle.org/mod/forum/discuss.php?d=221403 - however, I will post additional screen shots here. When testing, it might be advisable to use the Moodle 2.4 branch as my master branch has a tendency to break with all the good work being carried out by Marina Glancy, but at the moment it works, so you can get a version for master from https://github.com/gjb2048/moodle-format_topcoll - or if that fails, use the latest 2.4 version from https://moodle.org/plugins/pluginversions.php?plugin=format_topcoll . However can I annotate what the changes are and why you will see noticeable difference in most cases but logic dictates that the code is redundant and inefficient: In custom.js (// comments at the end): //collapsed topic only stuff $('div#page-course-view-topcollPAGE').live('pagebeforecreate',function(event, ui){ // Overall selector that applies to Collapsed Topics only courses / pages. $('#page-course-view-topcollPAGE ul.section').attr( "data-role" , "none" ); // Causes issue as seen on https://moodle.org/mod/forum/discuss.php?d=221403 $('.section li img').removeClass( "ui-li-icon" ); // I'm told by Michele Turre that this causes an issue. $.getScript('../course/format/topcoll/module.js'); // Does nothing. $('#page-course-view-topcollPAGE tr.cps a').attr( "data-role" , "button" ).attr( "data-icon" , "arrow-r" ); // Selector died with introduction of Moodle 2.3 version of Collapsed Topics so redundant. $('#page-course-view-topcollPAGE #thetopics').attr( "data-role" , "controlgroup" ); // Selector died with introduction of Moodle 2.3 version of Collapsed Topics so redundant. $('#page-course-view-topcollPAGE td.cps_centre').each(function(index) { // Selector died with introduction of Moodle 2.3 version of Collapsed Topics so redundant. var cpsc = $( this ).text().replace('<br>','').replace(')',''); $( this ).prev('td').find('a').append('<span class= "ui-li-count ui-btn-up-a ui-btn-corner-all" >' + cpsc + '</span>'); // Selector died with introduction of Moodle 2.3 version of Collapsed Topics so redundant. }); }); In 'core.css' (again with /* */ comments at the end of the line): /*collapsed topic format*/ #thetopics { /* Selector died with introduction of Moodle 2.3 version of Collapsed Topics so redundant. */ table-layout: inherit !important; width: 100%; display: block !important; } #page-course-view-topcollPAGE .section td.content, col.content { /* Selector died with introduction of Moodle 2.3 version of Collapsed Topics so redundant. */ text-align: left; width: 100% !important; overflow: hidden; } #page-course-view-topcollPAGE .section td.content { /* Selector died with introduction of Moodle 2.3 version of Collapsed Topics so redundant. */ padding: .5em; } #page-course-view-topcollPAGE td.left.side, #page-course-view-topcollPAGE td.right.side { /* Selector died with introduction of Moodle 2.3 version of Collapsed Topics so redundant. */ display: none; } #page-course-view-topcollPAGE tr.cps td a { /* Selector died with introduction of Moodle 2.3 version of Collapsed Topics so redundant. */ background: none !important; color: inherit !important; padding: 7px 0 7px 0px !important; } tr.cps td span { /* Selector died with introduction of Moodle 2.3 version of Collapsed Topics so redundant. */ font-style: inherit !important; font-size: 1.12em !important; } #page-course-view-topcollPAGE .section.separator, td.cps_centre { /* Selector died with introduction of Moodle 2.3 version of Collapsed Topics so redundant. */ display: none; } tr.cps { /* Selector died with introduction of Moodle 2.3 version of Collapsed Topics so redundant. */ background-color: inherit !important; color: inherit !important; } .opencps .ui-btn- inner .ui-icon-arrow-r { /* Has no effect as toggles not currently rendered by jQueryMobile - this is my issue to fix in CONTRIB-3655. */ background-position: -216px 50%; } tr.cps td span.ui-li-count { /* Selector died with introduction of Moodle 2.3 version of Collapsed Topics so redundant. */ font-size: .6em !important; right: 12px; } #page-course-view-topcollPAGE .cps .ui-btn- inner { /* Has no effect as toggles not currently rendered by jQueryMobile - this is my issue to fix in CONTRIB-3655. */ white-space: normal; } Screen shots and testing instructions to follow. Cheers, Gareth
          Hide
          Mary Evans added a comment - - edited

          Hi Dan,

          Moodle Coding Guidelines says that line length should be 140 (I think it is 140) so Text Pad (my Text Editor) is set up to do just that on save. Hence the short lines.

          I was not aware line length can have so much affect when it comes to JavaScript! Goes to show how much one can learn from working on fixing these Moodle bugs.

          Now I know where I have gone wrong I'll fix it and resubmit.

          Thanks

          Show
          Mary Evans added a comment - - edited Hi Dan, Moodle Coding Guidelines says that line length should be 140 (I think it is 140) so Text Pad (my Text Editor) is set up to do just that on save. Hence the short lines. I was not aware line length can have so much affect when it comes to JavaScript! Goes to show how much one can learn from working on fixing these Moodle bugs. Now I know where I have gone wrong I'll fix it and resubmit. Thanks
          Hide
          Gareth J Barnard added a comment -

          Dear Mary,

          Code checker - https://moodle.org/plugins/view.php?plugin=local_codechecker - has it at 132 - I did not know that until very recently

          Cheers,

          Gareth

          Show
          Gareth J Barnard added a comment - Dear Mary, Code checker - https://moodle.org/plugins/view.php?plugin=local_codechecker - has it at 132 - I did not know that until very recently Cheers, Gareth
          Hide
          Gareth J Barnard added a comment -

          Test course 'backup-moodle2-course-2-ct-20130205-0959-nu.mbz' for testing instructions.

          Show
          Gareth J Barnard added a comment - Test course 'backup-moodle2-course-2-ct-20130205-0959-nu.mbz' for testing instructions.
          Hide
          Gareth J Barnard added a comment -

          Screen shot 'MDL-37749_before_change_before_login.PNG'.

          Show
          Gareth J Barnard added a comment - Screen shot ' MDL-37749 _before_change_before_login.PNG'.
          Hide
          Gareth J Barnard added a comment -

          Screen shot 'MDL-37749_before_change_topics.PNG' with test course's format set at 'Topics'.

          Show
          Gareth J Barnard added a comment - Screen shot ' MDL-37749 _before_change_topics.PNG' with test course's format set at 'Topics'.
          Hide
          Gareth J Barnard added a comment -

          Screen shot 'MDL-37749_before_change_topcoll.PNG' with test course's format as 'Collapsed Topics'.

          Show
          Gareth J Barnard added a comment - Screen shot ' MDL-37749 _before_change_topcoll.PNG' with test course's format as 'Collapsed Topics'.
          Hide
          Gareth J Barnard added a comment -

          Screen shot 'MDL-37749_after_change_before_login.PNG' after change.

          Show
          Gareth J Barnard added a comment - Screen shot ' MDL-37749 _after_change_before_login.PNG' after change.
          Hide
          Gareth J Barnard added a comment -

          Screen shot 'MDL-37749_after_change_topics.PNG' after the change with the test course's format set at 'Topics'.

          Show
          Gareth J Barnard added a comment - Screen shot ' MDL-37749 _after_change_topics.PNG' after the change with the test course's format set at 'Topics'.
          Hide
          Gareth J Barnard added a comment -

          Screen shot 'MDL-37749_after_change_topcoll.PNG' after the change with the test course's format set at 'Topics'.

          Show
          Gareth J Barnard added a comment - Screen shot ' MDL-37749 _after_change_topcoll.PNG' after the change with the test course's format set at 'Topics'.
          Hide
          Dan Poltawski added a comment -

          Hi Gareth/Mary,

          On the code checker, its great to make our code conform to it, but because there is a risk that any change can cause bugs, we try to avoid making these 'asthetic' changes on the stable branches and only fix the bug in question.

          We reserve that kind of clean up for master.

          Show
          Dan Poltawski added a comment - Hi Gareth/Mary, On the code checker, its great to make our code conform to it, but because there is a risk that any change can cause bugs, we try to avoid making these 'asthetic' changes on the stable branches and only fix the bug in question. We reserve that kind of clean up for master.
          Hide
          Gareth J Barnard added a comment -

          Testing instructions.

          With the Moodle master branch, Collapsed Topics from (https://github.com/gjb2048/moodle-format_topcoll or use the 2.4 version from https://moodle.org/plugins/pluginversions.php?plugin=format_topcoll) installed and the test course 'backup-moodle2-course-2-ct-20130205-0959-nu':

          1. With iPod Touch or other device / simulator confirm that the pre-login screen is as screen shot 'MDL-37749_before_change_before_login.PNG'.
          2. Ensure that the test course (From 'backup-moodle2-course-2-ct-20130205-0959-nu.mbz' or own similar) set to the 'Topics' format.
          3. With iPod Touch or other device / simulator, login and go to the test course, and confirm that the screen is as screen shot 'MDL-37749_before_change_topics.PNG'.
          4. Change the course format of the test course to 'Collapsed Topics' via the 'Edit settings' option of the course.
          5. With iPod Touch or other device / simulator, refresh the test course, and confirm that the screen is as screen shot 'MDL-37749_before_change_topcoll.PNG'.
          6. Logout of Moodle, perform the code changes and do a 'Purge all caches'.
          7. With iPod Touch or other device / simulator confirm that the pre-login screen is as screen shot 'MDL-37749_after_change_before_login.PNG'.
          8. Change the course format of the test course to 'Topics' via the 'Edit settings' option of the course.
          9. With iPod Touch or other device / simulator, login and go to the test course, and confirm that the screen is as screen shot 'MDL-37749_after_change_topics.PNG'.
          10. Change the course format of the test course to 'Collapsed Topics' via the 'Edit settings' option of the course.
          11. With iPod Touch or other device / simulator, refresh the test course, and confirm that the screen is as screen shot 'MDL-37749_after_change_topcoll.PNG'.

          Cheers,

          Gareth

          Show
          Gareth J Barnard added a comment - Testing instructions. With the Moodle master branch, Collapsed Topics from ( https://github.com/gjb2048/moodle-format_topcoll or use the 2.4 version from https://moodle.org/plugins/pluginversions.php?plugin=format_topcoll ) installed and the test course 'backup-moodle2-course-2-ct-20130205-0959-nu': 1. With iPod Touch or other device / simulator confirm that the pre-login screen is as screen shot ' MDL-37749 _before_change_before_login.PNG'. 2. Ensure that the test course (From 'backup-moodle2-course-2-ct-20130205-0959-nu.mbz' or own similar) set to the 'Topics' format. 3. With iPod Touch or other device / simulator, login and go to the test course, and confirm that the screen is as screen shot ' MDL-37749 _before_change_topics.PNG'. 4. Change the course format of the test course to 'Collapsed Topics' via the 'Edit settings' option of the course. 5. With iPod Touch or other device / simulator, refresh the test course, and confirm that the screen is as screen shot ' MDL-37749 _before_change_topcoll.PNG'. 6. Logout of Moodle, perform the code changes and do a 'Purge all caches'. 7. With iPod Touch or other device / simulator confirm that the pre-login screen is as screen shot ' MDL-37749 _after_change_before_login.PNG'. 8. Change the course format of the test course to 'Topics' via the 'Edit settings' option of the course. 9. With iPod Touch or other device / simulator, login and go to the test course, and confirm that the screen is as screen shot ' MDL-37749 _after_change_topics.PNG'. 10. Change the course format of the test course to 'Collapsed Topics' via the 'Edit settings' option of the course. 11. With iPod Touch or other device / simulator, refresh the test course, and confirm that the screen is as screen shot ' MDL-37749 _after_change_topcoll.PNG'. Cheers, Gareth
          Hide
          Mary Evans added a comment -

          Thanks Gareth for the alert this morning.
          I'm working on this right now.

          I've set up my Text Editor to work differently with JavaScript so it's behaving itself now.

          Just done the M23 branch will fix M24 & master shortly.

          Thanks again
          M

          Show
          Mary Evans added a comment - Thanks Gareth for the alert this morning. I'm working on this right now. I've set up my Text Editor to work differently with JavaScript so it's behaving itself now. Just done the M23 branch will fix M24 & master shortly. Thanks again M
          Hide
          Mary Evans added a comment -

          Re-submitting for Integration Review

          Show
          Mary Evans added a comment - Re-submitting for Integration Review
          Hide
          Damyon Wiese added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          Cheers!

          Show
          Damyon Wiese added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. Cheers!
          Hide
          Mary Evans added a comment -

          REBASED ALL BRANCHES

          Show
          Mary Evans added a comment - REBASED ALL BRANCHES
          Hide
          Gareth J Barnard added a comment - - edited

          Dear Mary,

          Really annoyed with myself as just found:

          .weeks .current .headingwrap, .topics .current div.headingwrap, .current .ui-bar-b, .current .left.side, .tabtree ul.tabrow0 li.selected a, .ui-btn-active, #page-course-view-topcollPAGE .current {
          

          In 'core.css' line 1457 with the last little selector '#page-course-view-topcollPAGE .current' to be removed.

          How very annoying at this stage. Currently kicking myself.

          I thought I had everything but there is code all over the place!

          Cheers,

          Gareth

          Show
          Gareth J Barnard added a comment - - edited Dear Mary, Really annoyed with myself as just found: .weeks .current .headingwrap, .topics .current div.headingwrap, .current .ui-bar-b, .current .left.side, .tabtree ul.tabrow0 li.selected a, .ui-btn-active, #page-course-view-topcollPAGE .current { In 'core.css' line 1457 with the last little selector '#page-course-view-topcollPAGE .current' to be removed. How very annoying at this stage. Currently kicking myself. I thought I had everything but there is code all over the place! Cheers, Gareth
          Hide
          Mary Evans added a comment - - edited

          Dear Gareth,
          Don't worry about it. It's easy to remove that selector, so no problem.
          I had seen that myself the other day when trying to clone the MyMobile theme for a client. It's a complicated theme. The CSS is a nightmare! All I wanted to do was change the color scheme. LOL

          If #page-course-view-topcollPAGE .current has to be removed then will i need to remove this <div id="<?php p($PAGE->bodyid) ?>PAGE" from this line in mymobile/layout/general.php...

          <div id="<?php p($PAGE->bodyid) ?>PAGE" data-role="page" class="generalpage <?php echo 'ajaxedclass '; p($PAGE->bodyclasses.' '.join(' ', $bodyclasses));  ?> <?php if ($hasmyblocks && $usercol) { echo 'has-myblocks'; } ?> " data-title="<?php p($SITE->shortname) ?>">
          

          ...that's assuming it is associated with the Collapsed Topics plugin?

          Show
          Mary Evans added a comment - - edited Dear Gareth, Don't worry about it. It's easy to remove that selector, so no problem. I had seen that myself the other day when trying to clone the MyMobile theme for a client. It's a complicated theme. The CSS is a nightmare! All I wanted to do was change the color scheme. LOL If #page-course-view-topcollPAGE .current has to be removed then will i need to remove this <div id="<?php p($PAGE->bodyid) ?>PAGE" from this line in mymobile/layout/general.php... <div id= "<?php p($PAGE->bodyid) ?>PAGE" data-role= "page" class= "generalpage <?php echo 'ajaxedclass '; p($PAGE->bodyclasses.' '.join(' ', $bodyclasses)); ?> <?php if ($hasmyblocks && $usercol) { echo 'has-myblocks'; } ?> " data-title= "<?php p($SITE->shortname) ?>" > ...that's assuming it is associated with the Collapsed Topics plugin?
          Hide
          Gareth J Barnard added a comment -

          Dear Mary,

          Thanks . I believe that:

          #<div id="<?php p($PAGE->bodyid) ?>PAGE"
          

          Is used by the custom.js as a means of parameter passing to the jQuery code.

          I'll PM you about the MyMobile theme.

          Cheers,

          Gareth

          Show
          Gareth J Barnard added a comment - Dear Mary, Thanks . I believe that: #<div id= "<?php p($PAGE->bodyid) ?>PAGE" Is used by the custom.js as a means of parameter passing to the jQuery code. I'll PM you about the MyMobile theme. Cheers, Gareth
          Hide
          Dan Poltawski added a comment -

          Hi Mary/Gareth,
          It looks like there are 2 different git commits with the same commit message and a merge commit for this change, so I think there was a problem in the rebasing. In fact this looks like the same sort of problem as in MDL-34798, did you make the same mistake?

          Reopening to sort out those commits. Thanks

          Show
          Dan Poltawski added a comment - Hi Mary/Gareth, It looks like there are 2 different git commits with the same commit message and a merge commit for this change, so I think there was a problem in the rebasing. In fact this looks like the same sort of problem as in MDL-34798 , did you make the same mistake? Reopening to sort out those commits. Thanks
          Hide
          CiBoT added a comment -

          Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

          Show
          CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
          Hide
          Gareth J Barnard added a comment -

          Dear Dan,

          Rebasing in principle seems easy, but when used in practice seems to be fraught with issues. Took me ages to sort MDL-34798 yesterday and then worked out a way of rebasing, squashing the commits, fixing the commit message then forced push on GitHub repo. So will create a small guide on how I did it with a combination of command line and TortoiseGit - as although I'm happy with the command line, drop downs and tick boxes are so much easier. Mary - I'll PM you the guide to try when it's done

          Cheers,

          Gareth

          Show
          Gareth J Barnard added a comment - Dear Dan, Rebasing in principle seems easy, but when used in practice seems to be fraught with issues. Took me ages to sort MDL-34798 yesterday and then worked out a way of rebasing, squashing the commits, fixing the commit message then forced push on GitHub repo. So will create a small guide on how I did it with a combination of command line and TortoiseGit - as although I'm happy with the command line, drop downs and tick boxes are so much easier. Mary - I'll PM you the guide to try when it's done Cheers, Gareth
          Hide
          Mary Evans added a comment -

          I knew I should have committed that last fix as a separate item! LOL

          Should be 3rd time lucky if it makes it the next time round...

          Show
          Mary Evans added a comment - I knew I should have committed that last fix as a separate item! LOL Should be 3rd time lucky if it makes it the next time round...
          Hide
          Mary Evans added a comment -

          Gareth, I don't use Tortoise Git, I use GitBash

          Show
          Mary Evans added a comment - Gareth, I don't use Tortoise Git, I use GitBash
          Hide
          Gareth J Barnard added a comment -

          Ok - Indeed GitBash is aptly named. I think Dan wants all of the commits squashed into a single commit

          Show
          Gareth J Barnard added a comment - Ok - Indeed GitBash is aptly named. I think Dan wants all of the commits squashed into a single commit
          Hide
          Mary Evans added a comment -

          @dan I made the mistake of acting on the instructions as given in GIT, that of...

          git pull origin wip-MDL-37749_master

          which does a FETCH_HEAD and merge all in one which I thought was great! I then pushed this merge back to origin with...

          git push origin wip-MDL-37749_master

          So the moral of this tale is don't follow GIT suggestions as these can cause conflict in Moodle

          I'll remember it as a Push-me-Pull-you from Dr.Dolittle

          Show
          Mary Evans added a comment - @dan I made the mistake of acting on the instructions as given in GIT, that of... git pull origin wip-MDL-37749_master which does a FETCH_HEAD and merge all in one which I thought was great! I then pushed this merge back to origin with... git push origin wip-MDL-37749_master So the moral of this tale is don't follow GIT suggestions as these can cause conflict in Moodle I'll remember it as a Push-me-Pull-you from Dr.Dolittle
          Hide
          Mary Evans added a comment -

          @ Gareth J Barnard I thought that was what I was doing I obviously lost the plot!

          The problem was I had re-based the original and then tried to add a new commit on top of the old which is fatal! I should have known better. I must admit I should have done as my instinct was telling me to do, that of creating a new branch and squashing the lot into it.

          Show
          Mary Evans added a comment - @ Gareth J Barnard I thought that was what I was doing I obviously lost the plot! The problem was I had re-based the original and then tried to add a new commit on top of the old which is fatal! I should have known better. I must admit I should have done as my instinct was telling me to do, that of creating a new branch and squashing the lot into it.
          Hide
          Gareth J Barnard added a comment -

          Mary Evans Your instinct is what I did yesterday on MDL-34798 but reapplied the source code changes (only ten lines overall per branch in two files). Until I worked out rebasing on the current branch with squash to make one commit and then amend the comment to ensure it's in the right format.

          Cheers,

          Gareth

          Show
          Gareth J Barnard added a comment - Mary Evans Your instinct is what I did yesterday on MDL-34798 but reapplied the source code changes (only ten lines overall per branch in two files). Until I worked out rebasing on the current branch with squash to make one commit and then amend the comment to ensure it's in the right format. Cheers, Gareth
          Hide
          Mary Evans added a comment -

          Hopefully squashed and fixed

          Show
          Mary Evans added a comment - Hopefully squashed and fixed
          Hide
          Mary Evans added a comment -

          Gareth I did the following for each branch, which is what Tim Hunt kindly explained how to do when I got into a similar fix in MDL-29723...

          git checkout -b MDL-37749_master
          git merge --squash wip-MDL-37749_master
          git commit
          
          Show
          Mary Evans added a comment - Gareth I did the following for each branch, which is what Tim Hunt kindly explained how to do when I got into a similar fix in MDL-29723 ... git checkout -b MDL-37749_master git merge --squash wip-MDL-37749_master git commit
          Hide
          Mary Evans added a comment - - edited

          To rebase a branches MDL-xxxxx_master, MDL-xxxxx_M24, and MDL-xxxxx_M23 you have to do...

          git rebase master MDL-xxxxx_master
          git rebase M00DLE_23_STABLE MDL-xxxxx_M23
          git rebase M00DLE_24_STABLE MDL-xxxxx_M24
          

          If you get ANY conflicts just do...

          git rebase --abort
          Show
          Mary Evans added a comment - - edited To rebase a branches MDL-xxxxx_master, MDL-xxxxx_M24, and MDL-xxxxx_M23 you have to do... git rebase master MDL-xxxxx_master git rebase M00DLE_23_STABLE MDL-xxxxx_M23 git rebase M00DLE_24_STABLE MDL-xxxxx_M24 If you get ANY conflicts just do... git rebase --abort
          Hide
          Mary Evans added a comment -

          In addition to my last comment before rebasing any commited branch you have to have updated all your Moodle branches which are currently M00DLE_23_STABLE, M00DLE_24_STABLE and master.

          Show
          Mary Evans added a comment - In addition to my last comment before rebasing any commited branch you have to have updated all your Moodle branches which are currently M00DLE_23_STABLE, M00DLE_24_STABLE and master.
          Hide
          Gareth J Barnard added a comment -

          Thanks Mary, so simple when put like this. Possibly a developer doc update needed . I was able to do:

          git rebase --continue
          

          When I resolved but did not commit conflicts. Happened twice but worked in the end.

          Show
          Gareth J Barnard added a comment - Thanks Mary, so simple when put like this. Possibly a developer doc update needed . I was able to do: git rebase -- continue When I resolved but did not commit conflicts. Happened twice but worked in the end.
          Hide
          Damyon Wiese added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          Thanks!

          Show
          Damyon Wiese added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. Thanks!
          Hide
          Mary Evans added a comment -

          Rebased again!

          Show
          Mary Evans added a comment - Rebased again!
          Hide
          Mary Evans added a comment -

          @ Gareth J Barnard

          It's not usual to have before and after instructions. For this to be simpler we only need to check that MyMobile works correctly. In other words, we need to show that removing the JS & CSS code for something that should not have gone into MyMobile theme in the first place, has not broken the theme, don't we?

          I think the less we make of this the quicker it will be fixed, then you can get on with building a compatible version for Collapsed Topics.

          Show
          Mary Evans added a comment - @ Gareth J Barnard It's not usual to have before and after instructions. For this to be simpler we only need to check that MyMobile works correctly. In other words, we need to show that removing the JS & CSS code for something that should not have gone into MyMobile theme in the first place, has not broken the theme, don't we? I think the less we make of this the quicker it will be fixed, then you can get on with building a compatible version for Collapsed Topics.
          Hide
          Gareth J Barnard added a comment -

          Dear Mary,

          The reality is that technically there is one line in the css that causes the theme to be broken when used by Collapsed Topics. But then the code should not have been in there in the first place given what Martin has said on https://moodle.org/mod/forum/discuss.php?d=218947#p953746 - therefore the code should not have been in there in the first place, needs to be removed and is my problem because any such code fix in a contributed add-on is not the responsibility or control of Moodle HQ to maintain.

          Must go now! Will check what I've said later .

          Cheers,

          Gareth

          Show
          Gareth J Barnard added a comment - Dear Mary, The reality is that technically there is one line in the css that causes the theme to be broken when used by Collapsed Topics. But then the code should not have been in there in the first place given what Martin has said on https://moodle.org/mod/forum/discuss.php?d=218947#p953746 - therefore the code should not have been in there in the first place, needs to be removed and is my problem because any such code fix in a contributed add-on is not the responsibility or control of Moodle HQ to maintain. Must go now! Will check what I've said later . Cheers, Gareth
          Hide
          Gareth J Barnard added a comment -

          Mary Evans I think I repeated myself above. Anyway, the case is that there should be no Collapsed Topics supporting code in core (unless generic API change) even if it breaks my code. Therefore this is a logic thing as code is reduced. If a test is required, then the proof is that nothing has changed to the theme before / after the fix when used with the standard course formats.

          Show
          Gareth J Barnard added a comment - Mary Evans I think I repeated myself above. Anyway, the case is that there should be no Collapsed Topics supporting code in core (unless generic API change) even if it breaks my code. Therefore this is a logic thing as code is reduced. If a test is required, then the proof is that nothing has changed to the theme before / after the fix when used with the standard course formats.
          Hide
          Mary Evans added a comment -

          Gareth
          I've just picked up some work John has done to fix MyMoodle in MDL-33934, there are a lot of changes and may affect this integration.

          Show
          Mary Evans added a comment - Gareth I've just picked up some work John has done to fix MyMoodle in MDL-33934 , there are a lot of changes and may affect this integration.
          Hide
          Gareth J Barnard added a comment -

          Mary Evans The only issues in MDL-33934 are in custom.js:

           $('div#page-course-view-topcollPAGE').live('pagecreate',function(event, ui){
                   $.getScript('../course/format/topcoll/module.js');
          

          and in media.css:

          .course-content ul.ctopics li.section .content, .course-content ul.ctopics li.tcsection .content {
              margin: 0 0px;
              padding: 0;
          }
          

          However, the slight change in custom.js where the event handler has changed might actually help me with a fix I'm working on where my toggles do not work - please see comments on MDL-33115 of 14/15 Feb

          Cheers,

          Gareth

          Show
          Gareth J Barnard added a comment - Mary Evans The only issues in MDL-33934 are in custom.js: $('div#page-course-view-topcollPAGE').live('pagecreate',function(event, ui){ $.getScript('../course/format/topcoll/module.js'); and in media.css: .course-content ul.ctopics li.section .content, .course-content ul.ctopics li.tcsection .content { margin: 0 0px; padding: 0; } However, the slight change in custom.js where the event handler has changed might actually help me with a fix I'm working on where my toggles do not work - please see comments on MDL-33115 of 14/15 Feb Cheers, Gareth
          Hide
          Mary Evans added a comment - - edited

          NOTE TO INTEGRATOR

          To aid in making these changes seemless and conflict free, because the major part of this fix will conflict with MDL-33934_master, I have removed the master branch from this MDL-37749 issue so that MOODLE_23_STABLE & MOODLE_24_STABLE can be Integrated seperately.

          I Have created MDL-38072 as a sub-task to remove Collapsed Topics' contributed plugin code js and CSS for the master branch, based on MDL-33934_master branch which is also set for Integration Review this week.

          Hope this is OK?

          Show
          Mary Evans added a comment - - edited NOTE TO INTEGRATOR To aid in making these changes seemless and conflict free, because the major part of this fix will conflict with MDL-33934 _master, I have removed the master branch from this MDL-37749 issue so that MOODLE_23_STABLE & MOODLE_24_STABLE can be Integrated seperately. I Have created MDL-38072 as a sub-task to remove Collapsed Topics' contributed plugin code js and CSS for the master branch, based on MDL-33934 _master branch which is also set for Integration Review this week. Hope this is OK?
          Hide
          Dan Poltawski added a comment -

          Integrated to 24 and 23, thanks Mary/Gareth.

          Damyon has the issue for master as hes integrating the blocked issue.

          Show
          Dan Poltawski added a comment - Integrated to 24 and 23, thanks Mary/Gareth. Damyon has the issue for master as hes integrating the blocked issue.
          Hide
          Rossiani Wijaya added a comment -

          Beside the missing bottom rounded corner that has been reported in MDL-35234, this is working as expected.

          *attaching image

          Tested this for 2.3 and 2.4 only.

          Test passed.

          Show
          Rossiani Wijaya added a comment - Beside the missing bottom rounded corner that has been reported in MDL-35234 , this is working as expected. *attaching image Tested this for 2.3 and 2.4 only. Test passed.
          Hide
          Mary Evans added a comment -

          Thanks Rossi.

          Show
          Mary Evans added a comment - Thanks Rossi.
          Hide
          Damyon Wiese added a comment -

          Thanks for your hard work - this issue has made it! Moodle is now a little bit better.

          Regards, Damyon

          Show
          Damyon Wiese added a comment - Thanks for your hard work - this issue has made it! Moodle is now a little bit better. Regards, Damyon
          Hide
          Mary Evans added a comment -

          I am reopening this so I can tag onto it the missing fixes for 2.5Dev

          Show
          Mary Evans added a comment - I am reopening this so I can tag onto it the missing fixes for 2.5Dev
          Hide
          Eloy Lafuente (stronk7) added a comment -

          (sorry but it's not recommended to reopen already closed issues. plz create new linked issues instead)

          Show
          Eloy Lafuente (stronk7) added a comment - (sorry but it's not recommended to reopen already closed issues. plz create new linked issues instead)
          Hide
          Mary Evans added a comment -

          Sorry Eloy I reopened this in error, and forgot to close it again!

          Show
          Mary Evans added a comment - Sorry Eloy I reopened this in error, and forgot to close it again!
          Hide
          Gareth J Barnard added a comment -

          Fixed

          Show
          Gareth J Barnard added a comment - Fixed

            People

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

              Dates

              • Created:
                Updated:
                Resolved: