Moodle
  1. Moodle
  2. MDL-33357

META: Update MyMobile theme inline with Moodle 2.3

    Details

    • Type: Task Task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3
    • Fix Version/s: 2.3
    • Component/s: Themes
    • Labels:
    • Testing Instructions:
      Hide
      1. Select MyMobile theme as the main theme for a Mobile (and/or Tablet) device.
      2. Using a mobile or tablet device, check that login to moodle and standard moodle mechanics work. Note that some admin tasks (like editing) are not available. But, you should be able to easily:
        • view a course
        • view forums
        • post to a forum
        • view resources.

      suggestion: feel free to start a test session for your mymobile theme instance here and share for others to create issues during the session ! a little explorative testing from other devices here wouldn't hurt i imagine.

      Show
      Select MyMobile theme as the main theme for a Mobile (and/or Tablet) device. Using a mobile or tablet device, check that login to moodle and standard moodle mechanics work. Note that some admin tasks (like editing) are not available. But, you should be able to easily: view a course view forums post to a forum view resources. suggestion: feel free to start a test session for your mymobile theme instance here and share for others to create issues during the session ! a little explorative testing from other devices here wouldn't hurt i imagine.
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:
    • Rank:
      41215

      Description

      As Moodle 2.3 gets ready to be rolled out, the need for MyMobile theme to keep inline with the new features available in Moodle 2.3 has become a necessity.

        Activity

        Hide
        Mary Evans added a comment - - edited

        @John
        I have just set up MDL-33357 as the parent for all the sub-tasks which need fixing.

        You can add to the description if you feel the need as things progress.

        Thanks
        Mary

        Show
        Mary Evans added a comment - - edited @John I have just set up MDL-33357 as the parent for all the sub-tasks which need fixing. You can add to the description if you feel the need as things progress. Thanks Mary
        Hide
        John Stabinger added a comment -

        Thanks Mary. Working on this now. The code has been update to jqm 1.1, the momentum scrolling (using JS) has been removed in favor of device native touch-overflow and I have been working on some bugs (some listed here, others not).

        Show
        John Stabinger added a comment - Thanks Mary. Working on this now. The code has been update to jqm 1.1, the momentum scrolling (using JS) has been removed in favor of device native touch-overflow and I have been working on some bugs (some listed here, others not).
        Hide
        Mary Evans added a comment -

        In that case John can you tell me what these MDL issue numbers are then I can add them?

        Thanks

        Show
        Mary Evans added a comment - In that case John can you tell me what these MDL issue numbers are then I can add them? Thanks
        Hide
        John Stabinger added a comment -

        Sorry for the confusion Mary, I did not mean that you missed issues here in the tracker. I was referring to small issues not in the tracker.

        Show
        John Stabinger added a comment - Sorry for the confusion Mary, I did not mean that you missed issues here in the tracker. I was referring to small issues not in the tracker.
        Hide
        Mary Evans added a comment -

        Well in that case let's ADD it. If you go to "More Actions" (drop-down menu in this page) and select Create Sub-Task, just fill in the form as normal. It will then show up on this page as No. 11 in the list.

        Cheers
        Mary

        Show
        Mary Evans added a comment - Well in that case let's ADD it. If you go to "More Actions" (drop-down menu in this page) and select Create Sub-Task, just fill in the form as normal. It will then show up on this page as No. 11 in the list. Cheers Mary
        Hide
        Mary Evans added a comment -

        Also if there is anything I can help you with like, doing some of the coding and pushing it to GITHUB, let me know and I will be happy to help you out.

        Show
        Mary Evans added a comment - Also if there is anything I can help you with like, doing some of the coding and pushing it to GITHUB, let me know and I will be happy to help you out.
        Hide
        John Stabinger added a comment -

        Hi Mary. I added the updated theme. This updates the theme to use the latest JQM code, uses touchoverflow instead of the old javascript scrolling and some small updates.

        In addition this also has fixes for sub tasks:

        1.
        3.
        4.
        5.
        9.
        10.

        Let me know what needs to happen next to get this into peer review. Thanks,

        John

        Show
        John Stabinger added a comment - Hi Mary. I added the updated theme. This updates the theme to use the latest JQM code, uses touchoverflow instead of the old javascript scrolling and some small updates. In addition this also has fixes for sub tasks: 1. 3. 4. 5. 9. 10. Let me know what needs to happen next to get this into peer review. Thanks, John
        Hide
        Mary Evans added a comment -

        Hi John,

        If you know someone who can peer review this for you then ask them and select their name when you fill in the Start Peer Review...and away you go!

        Perhaps Julian Ridden or one of the other developers could do this?

        Alternatively, providing this is using the most up-to-date Moodle 'master' and you are confident MyMoible works, I would be tempted to set it for Integration Review as it would get tested anyway.

        Show
        Mary Evans added a comment - Hi John, If you know someone who can peer review this for you then ask them and select their name when you fill in the Start Peer Review...and away you go! Perhaps Julian Ridden or one of the other developers could do this? Alternatively, providing this is using the most up-to-date Moodle 'master' and you are confident MyMoible works, I would be tempted to set it for Integration Review as it would get tested anyway.
        Hide
        John Stabinger added a comment -

        This is using master. It works, better than the old version did. I'm concerned about some formatting (the source JQM files are not formatted to the moodle standard) so I don't know if that will be a block. Go ahead and set for integration review (if you don't mind!) and we'll see if anyone has an issue in testing. Thanks for your help and feedback.

        Show
        John Stabinger added a comment - This is using master. It works, better than the old version did. I'm concerned about some formatting (the source JQM files are not formatted to the moodle standard) so I don't know if that will be a block. Go ahead and set for integration review (if you don't mind!) and we'll see if anyone has an issue in testing. Thanks for your help and feedback.
        Hide
        Mary Evans added a comment -

        Set for integration.

        Can you add some TESTING INSTRUCTIONS John?

        Show
        Mary Evans added a comment - Set for integration. Can you add some TESTING INSTRUCTIONS John?
        Hide
        John Stabinger added a comment -

        General testing instructions are to select the theme as the mobile theme (or tablet theme). Then, using a mobile or tablet device, check that login to moodle and standard moodle mechanics work. Note that some admin tasks (like editing) are not available. But, you should be able to easily view a course, view forums, post to a forum, view resources.

        Should I add testing instructions here for each of the issue fixed above, or add them to that issue?

        Show
        John Stabinger added a comment - General testing instructions are to select the theme as the mobile theme (or tablet theme). Then, using a mobile or tablet device, check that login to moodle and standard moodle mechanics work. Note that some admin tasks (like editing) are not available. But, you should be able to easily view a course, view forums, post to a forum, view resources. Should I add testing instructions here for each of the issue fixed above, or add them to that issue?
        Hide
        Aparup Banerjee added a comment -

        Hi John,
        all the changes are under just one single commit. I think we can work with test instructions here in this one issue. my guess is the test will be split up in the way the subtasks have been.

        Show
        Aparup Banerjee added a comment - Hi John, all the changes are under just one single commit. I think we can work with test instructions here in this one issue. my guess is the test will be split up in the way the subtasks have been.
        Hide
        Mary Evans added a comment -

        Sorry John, missed adding the test instructions to the top section of this page - now done!

        Show
        Mary Evans added a comment - Sorry John, missed adding the test instructions to the top section of this page - now done!
        Hide
        Aparup Banerjee added a comment -

        Hi John,

        overall i think this is great.

        I've attached a *smurf.xml file that details some problems with phpdoc and mainly whitespace/tab issues that need to be addressed.

        theres also this -

        • theme/mymobile/renderers.php
          line 367 - get_context_instance() is the old call, use context_system::instance().
        • $.getScript('../course/format/topcoll/module.js');

        where are these files? oh they seem to be from a non-core course format
        maybe the needed snippets could be pulled. this bit would need further considerations.

        ps: feel free to add more to commit messages, like customcss being in here now.

        Show
        Aparup Banerjee added a comment - Hi John, overall i think this is great. I've attached a *smurf.xml file that details some problems with phpdoc and mainly whitespace/tab issues that need to be addressed. theres also this - theme/mymobile/renderers.php line 367 - get_context_instance() is the old call, use context_system::instance(). $.getScript('../course/format/topcoll/module.js'); where are these files? oh they seem to be from a non-core course format maybe the needed snippets could be pulled. this bit would need further considerations. ps: feel free to add more to commit messages, like customcss being in here now.
        Hide
        John Stabinger added a comment -

        Yes, the getscript is for the topcol fix. It is only called if the format is actually used, so for non-users it will never get called.

        How do I use the xml file that you posted? Thanks for your help,

        John

        Show
        John Stabinger added a comment - Yes, the getscript is for the topcol fix. It is only called if the format is actually used, so for non-users it will never get called. How do I use the xml file that you posted? Thanks for your help, John
        Hide
        Mary Evans added a comment - - edited

        The easiest way is open it with an xml editor and then read it. It's quite comprehensive, and details some minor problems with the code in lib.php which is mainly about commented lines, how they should be written. Also in renderers.php, and some in the css files too. These are mainly white spaces which need to be addressed before it gets integrated, like tabs instead of spaces, that sort of thing, or indents in php code, where an indent should be 8 spaces but found only 2.

        It may sound pedantic...but it's very important.

        There is a Code Checker in the plugins database.

        http://moodle.org/plugins/view.php?plugin=local_codechecker

        I use the one Tim Hunt made but I think there is another by Marina Glancy

        Hope this helps?

        Show
        Mary Evans added a comment - - edited The easiest way is open it with an xml editor and then read it. It's quite comprehensive, and details some minor problems with the code in lib.php which is mainly about commented lines, how they should be written. Also in renderers.php, and some in the css files too. These are mainly white spaces which need to be addressed before it gets integrated, like tabs instead of spaces, that sort of thing, or indents in php code, where an indent should be 8 spaces but found only 2. It may sound pedantic...but it's very important. There is a Code Checker in the plugins database. http://moodle.org/plugins/view.php?plugin=local_codechecker I use the one Tim Hunt made but I think there is another by Marina Glancy Hope this helps?
        Hide
        John Stabinger added a comment -

        Thanks, I just updated the pull info and branch info with an updated and corrected version.

        Show
        John Stabinger added a comment - Thanks, I just updated the pull info and branch info with an updated and corrected version.
        Hide
        Mary Evans added a comment - - edited

        John,

        I have just noticed that in embedded.php you have

        <?php echo core_renderer::MAIN_CONTENT_TOKEN ?>

        which will need to be changed to

        <?php echo $OUTPUT->main_content() ?>
        Show
        Mary Evans added a comment - - edited John, I have just noticed that in embedded.php you have <?php echo core_renderer::MAIN_CONTENT_TOKEN ?> which will need to be changed to <?php echo $OUTPUT->main_content() ?>
        Hide
        John Stabinger added a comment -

        I changed that late, but did not do so this time. Ugh. Will re-submit in a minute.

        Show
        John Stabinger added a comment - I changed that late, but did not do so this time. Ugh. Will re-submit in a minute.
        Hide
        John Stabinger added a comment -

        Update three is now linked above.

        Show
        John Stabinger added a comment - Update three is now linked above.
        Hide
        Aparup Banerjee added a comment -

        Thanks John i've integrated that along with a lot of whitespace fixes.

        a few tips i thought i'd mention for next time -

        • use an editor that takes care of trailing whitespaces for you when you save. i know netbeans does but it might be too heavy an editor for you.
        • feel free to break down you commits and changes into separated git commits. its alot easier to understand, follow and review. it also makes it easier for others to help you out .

        not sure why this was affect versions = 2.2.3 , changed to 2.3 (now title and that makes sense too)

        thanks a lot for this!

        ps: i've just noticed a weird quirk in my iphone4s - sometimes upon logging in the page width gets shortened - seems something to do with my keyboard moving out of the way when i hit done on the screen keyboard. it might be my phone :-|

        Show
        Aparup Banerjee added a comment - Thanks John i've integrated that along with a lot of whitespace fixes. a few tips i thought i'd mention for next time - use an editor that takes care of trailing whitespaces for you when you save. i know netbeans does but it might be too heavy an editor for you. feel free to break down you commits and changes into separated git commits. its alot easier to understand, follow and review. it also makes it easier for others to help you out . not sure why this was affect versions = 2.2.3 , changed to 2.3 (now title and that makes sense too) thanks a lot for this! ps: i've just noticed a weird quirk in my iphone4s - sometimes upon logging in the page width gets shortened - seems something to do with my keyboard moving out of the way when i hit done on the screen keyboard. it might be my phone :-|
        Hide
        Ankit Agarwal added a comment - - edited

        Hi Guys,
        Found following issues while testing with Android ICS:-

        1. on front page> click on blocks> the block "course description" shows up as blank tab. Clicking on the tab shows the description.
        2. Everything seems to be extremely slow and laggy
        3. When you click on "Settings" it shows the loading icon, and than it takes you to your existing page, before taking to the settings page
        4. Can be my phone but orientation changes were not properly detected.
        5. while editing a book resource, the form fields overlapped with the navigation menu (see the attached pic)
        6. Section pages are really messed up specially when the section doesn't contain any activities or resources.
        7. Drag and drop arrow is completely misplaced.
        8. I was trying to make a attachment to a forum post. Once I opened a filepicker, clicking on x wont close it.
        9. I had "student can manually mark this activity as complete" as the completion criteria for a file resource. I was able to mark the activity as complete from my phone. But it still shows as "uncomplete" when I access the site via desktop.
        10. View a book resource > goto the last chapter > click the up arrow. It should take you to course home page. But nothing really happens.
        11. Goto Quick grading page for any 2.3 assignment. Clicking on quick grading checkbox does nothing.
        12. Goto Quick grading page for any 2.3 assignment. Help icon for "quick grading" cannot be clicked.
        13. Randomly at times I was shown a blank page with just top navigation . Cannot determine the exact steps to reproduce it, but happened a few times.

        Please advise if I should pass/fail this test.
        I will create a bug report to deal with all these issues.

        Thanks

        Show
        Ankit Agarwal added a comment - - edited Hi Guys, Found following issues while testing with Android ICS:- on front page> click on blocks> the block "course description" shows up as blank tab. Clicking on the tab shows the description. Everything seems to be extremely slow and laggy When you click on "Settings" it shows the loading icon, and than it takes you to your existing page, before taking to the settings page Can be my phone but orientation changes were not properly detected. while editing a book resource, the form fields overlapped with the navigation menu (see the attached pic) Section pages are really messed up specially when the section doesn't contain any activities or resources. Drag and drop arrow is completely misplaced. I was trying to make a attachment to a forum post. Once I opened a filepicker, clicking on x wont close it. I had "student can manually mark this activity as complete" as the completion criteria for a file resource. I was able to mark the activity as complete from my phone. But it still shows as "uncomplete" when I access the site via desktop. View a book resource > goto the last chapter > click the up arrow. It should take you to course home page. But nothing really happens. Goto Quick grading page for any 2.3 assignment. Clicking on quick grading checkbox does nothing. Goto Quick grading page for any 2.3 assignment. Help icon for "quick grading" cannot be clicked. Randomly at times I was shown a blank page with just top navigation . Cannot determine the exact steps to reproduce it, but happened a few times. Please advise if I should pass/fail this test. I will create a bug report to deal with all these issues. Thanks
        Hide
        John Stabinger added a comment -

        Ankit,

        While I am not see the performance issues with my ICS tablet, I have seen some with other ICS devices. It seems related to the fixed header/touchoverflow so I will remove that and re-submit. I guess that spec just isn't ready for primetime yet. It can be a little buggy in IOS too (see Aparup's comment). I will re-submit with this feature removed in a minute.

        Other items-
        Book module- book module was not a core module when developed, so I have not checked this at all. I realize it is now, but updates will take some time.

        Grading- course/site admin is not really supported by the theme (as noted before and as indicated in the original version). I do not know when this will change, but not in the near term.

        Show
        John Stabinger added a comment - Ankit, While I am not see the performance issues with my ICS tablet, I have seen some with other ICS devices. It seems related to the fixed header/touchoverflow so I will remove that and re-submit. I guess that spec just isn't ready for primetime yet. It can be a little buggy in IOS too (see Aparup's comment). I will re-submit with this feature removed in a minute. Other items- Book module- book module was not a core module when developed, so I have not checked this at all. I realize it is now, but updates will take some time. Grading- course/site admin is not really supported by the theme (as noted before and as indicated in the original version). I do not know when this will change, but not in the near term.
        Hide
        Aparup Banerjee added a comment -

        Thanks for the awesome testing Ankit! Let's see how much we can get this polished this up.

        ps: Last i recall, mymobile is good for a student's browsing experience. any administrative sort of work will have unwanted consequences.

        Show
        Aparup Banerjee added a comment - Thanks for the awesome testing Ankit! Let's see how much we can get this polished this up. ps: Last i recall, mymobile is good for a student's browsing experience. any administrative sort of work will have unwanted consequences.
        Hide
        John Stabinger added a comment -

        Take 4 is now linked above. The fixed header/touchoverflow is removed. Sorry for the multiple attempts, but I want it to be as good as possible.

        Show
        John Stabinger added a comment - Take 4 is now linked above. The fixed header/touchoverflow is removed. Sorry for the multiple attempts, but I want it to be as good as possible.
        Hide
        Aparup Banerjee added a comment -

        Hi John, could you please make any fixes in separate commits, your branch has too many different changes to be grouped into a single commit.

        • it helps when integrating in a series of commits.
        • we can pick out your changes easily.
        • your continued effort is seen with the many commits going through.
        Show
        Aparup Banerjee added a comment - Hi John, could you please make any fixes in separate commits, your branch has too many different changes to be grouped into a single commit. it helps when integrating in a series of commits. we can pick out your changes easily. your continued effort is seen with the many commits going through.
        Hide
        Aparup Banerjee added a comment -

        Ankit, the patch really seems to help with lots of issues, so i'm still slightly inclined towards keeping this patch.

        have a look at the issues that were there before this patch from John. (i've pm-ed you a link to my branch without the patch for testing)

        Show
        Aparup Banerjee added a comment - Ankit, the patch really seems to help with lots of issues, so i'm still slightly inclined towards keeping this patch. have a look at the issues that were there before this patch from John. (i've pm-ed you a link to my branch without the patch for testing)
        Hide
        Aparup Banerjee added a comment -

        just to be clear, 'take 4' - https://github.com/epsd/moodle/compare/master...MDL-33357-4 hasn't been integrated yet.
        so far we're still at - https://github.com/epsd/moodle/compare/master...MDL-33357-3 + whitespace fixes.

        Show
        Aparup Banerjee added a comment - just to be clear, 'take 4' - https://github.com/epsd/moodle/compare/master...MDL-33357-4 hasn't been integrated yet. so far we're still at - https://github.com/epsd/moodle/compare/master...MDL-33357-3 + whitespace fixes.
        Hide
        Ankit Agarwal added a comment -

        So far looks good to me. Altough it has a few issues as I mentioned earlier. But it is way more crappy without the patch.
        My +1 to pass this issue and keep polishing the theme after wards. Am creating a sub-task to this issue with all the things I found.
        thanks

        Show
        Ankit Agarwal added a comment - So far looks good to me. Altough it has a few issues as I mentioned earlier. But it is way more crappy without the patch. My +1 to pass this issue and keep polishing the theme after wards. Am creating a sub-task to this issue with all the things I found. thanks
        Hide
        Martin Dougiamas added a comment -

        How much of this commit actually fixes all the subtasks? Can they be closed accordingly to make this less confusing? thanks.

        Show
        Martin Dougiamas added a comment - How much of this commit actually fixes all the subtasks? Can they be closed accordingly to make this less confusing? thanks.
        Hide
        Aparup Banerjee added a comment -

        John,
        can you close off the subtasks here that were fixed within the 'take 3' commit (https://github.com/epsd/moodle/commit/256c51fab0eb3a28f01672add3422c1c3fb89fea).
        i think atleast subtask 1 has been fixed here.

        We can then group the remaining sub-tasks into another meta along with Ankit's issues in a another meta.

        Show
        Aparup Banerjee added a comment - John, can you close off the subtasks here that were fixed within the 'take 3' commit ( https://github.com/epsd/moodle/commit/256c51fab0eb3a28f01672add3422c1c3fb89fea ). i think atleast subtask 1 has been fixed here. We can then group the remaining sub-tasks into another meta along with Ankit's issues in a another meta.
        Hide
        John Stabinger added a comment -

        Here are the sub tasks that were corrected (in take 3 and take 4):

        MDL-31474
        MDL-32047
        MDL-32389
        MDL-31346
        MDL-31347

        I can go in and close them if you would like.

        Show
        John Stabinger added a comment - Here are the sub tasks that were corrected (in take 3 and take 4): MDL-31474 MDL-32047 MDL-32389 MDL-31346 MDL-31347 I can go in and close them if you would like.
        Hide
        Mary Evans added a comment -

        I've just closed them John. Thanks

        Show
        Mary Evans added a comment - I've just closed them John. Thanks
        Hide
        Aparup Banerjee added a comment -

        i'm assuming that take 3 and take 4 had the same subtasks, right?

        Show
        Aparup Banerjee added a comment - i'm assuming that take 3 and take 4 had the same subtasks, right?
        Hide
        Eloy Lafuente (stronk7) added a comment -

        And this has been incorporated to all the weekly builds and also, to Moodle 2.3 Release Candidate 1, yay!

        Many, many thanks for your hard work!

        Ciao

        Show
        Eloy Lafuente (stronk7) added a comment - And this has been incorporated to all the weekly builds and also, to Moodle 2.3 Release Candidate 1, yay! Many, many thanks for your hard work! Ciao

          People

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

            Dates

            • Created:
              Updated:
              Resolved: