Moodle
  1. Moodle
  2. MDL-27622

Integrate MyMobile theme into core

    Details

    • Type: Task Task
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 2.0
    • Fix Version/s: 2.2
    • Component/s: Themes
    • Labels:
      None
    • Rank:
      17289

      Description

      John Stabinger has developed a jquerymobile-based theme for Moodle 2.x.

      http://mymobile.stabinger.us/
      http://moodle.org/mod/data/view.php?d=26&rid=4757
      http://moodle.org/mod/forum/discuss.php?d=174104

      This looks like an excellent candidate for core, especially in conjunction with MDL-25394.

      1. checkbox.png
        21 kB
      2. crops.png
        48 kB
      3. editbutton.png
        8 kB
      4. radios.png
        26 kB
      5. screenshot from Andrew.png
        86 kB

        Issue Links

          Activity

          Hide
          John Stabinger added a comment -

          That would be awesome Martin! I'd like to get the theme to a more stable beta stage, hopefully this will be accomplished in the first half of June. Looks like 2.1 is scheduled for June 30, so assuming that holds, it could potentially be ready by then.

          Show
          John Stabinger added a comment - That would be awesome Martin! I'd like to get the theme to a more stable beta stage, hopefully this will be accomplished in the first half of June. Looks like 2.1 is scheduled for June 30, so assuming that holds, it could potentially be ready by then.
          Hide
          Martin Dougiamas added a comment -

          How are you going? We really need something ASAP if this is going to be integrated in time for testing.

          Show
          Martin Dougiamas added a comment - How are you going? We really need something ASAP if this is going to be integrated in time for testing.
          Hide
          John Stabinger added a comment -

          I don't think it will be ready for 2.1. The beta version of the JS is not out yet, and it is really needed in order to support more devices and fix some issues. Even if it came out today, it would take some time to integrate it into the theme as there are some major refactors. I will update as soon as I know more.

          Show
          John Stabinger added a comment - I don't think it will be ready for 2.1. The beta version of the JS is not out yet, and it is really needed in order to support more devices and fix some issues. Even if it came out today, it would take some time to integrate it into the theme as there are some major refactors. I will update as soon as I know more.
          Hide
          Dongsheng Cai added a comment -

          From what I see in github repo, it's already bete 1 pre: https://github.com/jquery/jquery-mobile/blob/master/version.txt

          Show
          Dongsheng Cai added a comment - From what I see in github repo, it's already bete 1 pre: https://github.com/jquery/jquery-mobile/blob/master/version.txt
          Hide
          Dongsheng Cai added a comment -
          Show
          Dongsheng Cai added a comment - jQuery mobile beta 1 released today: http://jquerymobile.com/blog/2011/06/20/jquery-mobile-beta-1-released/
          Hide
          John Stabinger added a comment -

          I saw that, was up late working on it... I've got it mostly integrated now, just have a few small issues to address. I may be able to have something ready by Friday.

          Show
          John Stabinger added a comment - I saw that, was up late working on it... I've got it mostly integrated now, just have a few small issues to address. I may be able to have something ready by Friday.
          Hide
          Dongsheng Cai added a comment -

          Thanks John!

          Show
          Dongsheng Cai added a comment - Thanks John!
          Hide
          moodle.com added a comment -

          This doesn't look like it's going to make it for 2.1. We'll aim for 2.1.1.

          Michael;

          Show
          moodle.com added a comment - This doesn't look like it's going to make it for 2.1. We'll aim for 2.1.1. Michael;
          Hide
          John Stabinger added a comment -

          Updated branch for various issues.

          Show
          John Stabinger added a comment - Updated branch for various issues.
          Hide
          Martin Dougiamas added a comment -

          John, can you write something about the quality of it? Is it ready for a peer review?

          Show
          Martin Dougiamas added a comment - John, can you write something about the quality of it? Is it ready for a peer review?
          Hide
          Marina Glancy added a comment -

          Hi, John.
          Martin asked me to look at your theme. I understand that it is not finished yet, but probably I have noticed something you did not see yourself.
          The overall look of the mobile theme is very nice and there a lot of great solutions for small screens.
          I did some code review and some testing and would like to share the results with you:

          formatting

          • in php files code should be reformatted to follow Moodle guidelines and in all files tabs to be replaced with spaces (not sure about guidelines on formatting css and js)
          • There is some commented or unused code, especially in renderers.php, function navigation_node
          • disclaimer and phpdocs are missing in the most of files
          • version.php is missing

          other things from code review

          • settings.php should use get_string in choices
          • layout/*.php: some tags (<link>, <meta>, maybe more) do not have /> in the end
          • layout/*.php: the output in tag attributes should be wrapped in p()
            examples:
            <div data-role="page" id="chatpage" data-fullscreen="true" data-title="<?php echo $SITE->shortname ?>">
            <div id="content2" data-role="page" data-title="<?php echo $SITE->shortname ?>">
          • layout/embedded.php
            use get_string for 'home' (although it's only shown as alt text and probably not visible on mobile)
            <a class="ui-btn-right" data-ajax="false" data-icon="home" href="<?php echo $CFG->wwwroot ?>" data-iconpos="notext">home</a>

          I also did some testing, not everything, of course:

          as admin:

          • I was not able to enrol users into course, install language and probably do a lot of other things.
            But, I guess, administration within mobile theme was not the goal, so I did not test further

          as a student:

          • When I press 'logout' nothing seem to happen (although I actually log out)
          • My website is located at http://localhost/master/, after I click the course name the address looks like:
            http://localhost/master/#/master/course/view.php?id=2
            which results in broken icons near activities names in normal browser. In mobile simulator it works ok
          • Navigation button/dropdown looks strange:
            the 'navigation' option does nothing (and my guess is that it was intended to lead to a page similar to 'Settings'). I would also exclude 'home' from standard navigation because there is another home button on the very top. The title on the button sometimes changes (not sure what was it done for). The items outside of active thread in navigation menu (f.e. My home, Participants, ...) are not shown and are not reachable at all. Actually this looks more like a breadcrumb than navigation.
          • the alerts/messages look like buttons (and have no link of course):
            <div data-role="button" data-icon="alert" data-theme="d" class="notifysuccess">
          • some forms (f.e. change password) have very narrow space for labels
          • I wonder if it is possible to hide the 'blocks' button when there are no other blocks on the page
          • when my page is a result of form submission (f.e. in the middle of taking the quiz), the buttons 'blocks' and 'settings' do not work
          • I did not have a chance to test on the real mobile phone, but the simulator shows dropdowns (under navigation, language selector and in activities questions) with really narrow lines. Would be very inconvenient on touch screen device
          • in the standard themes there is a button with course name in the very bottom of the page. It is missing in mymobile theme
          • the course descriptions are missing from the home page (maybe intentionally)
          • hints do not work

          I backed up a 'features demo' course from qa.moodle.net and restored it into the site with mymobile theme. So I basically tried the different activities as a student. I should tell that a lot do work and look very nice with those big buttons. Some things, unfortunately do not work as expected:

          • file upload screen not themed
          • in 'advanced uploading of files' the Notes edit button does not work
          • checkboxes and radio buttons look strange (see screenshots)
          • quizes use alerts, which are not themed
          • images embedded in html text are not shown in the question texts
          • edit icon is missing in comments (see screenshot)

          What I want to say that it is a huge task and great work has been done. I'm sure after testing and fixing of the the small issues users will appreciate it very much!

          Marina

          Show
          Marina Glancy added a comment - Hi, John. Martin asked me to look at your theme. I understand that it is not finished yet, but probably I have noticed something you did not see yourself. The overall look of the mobile theme is very nice and there a lot of great solutions for small screens. I did some code review and some testing and would like to share the results with you: formatting in php files code should be reformatted to follow Moodle guidelines and in all files tabs to be replaced with spaces (not sure about guidelines on formatting css and js) There is some commented or unused code, especially in renderers.php, function navigation_node disclaimer and phpdocs are missing in the most of files version.php is missing other things from code review settings.php should use get_string in choices layout/*.php: some tags (<link>, <meta>, maybe more) do not have /> in the end layout/*.php: the output in tag attributes should be wrapped in p() examples: <div data-role="page" id="chatpage" data-fullscreen="true" data-title="<?php echo $SITE->shortname ?>"> <div id="content2" data-role="page" data-title="<?php echo $SITE->shortname ?>"> layout/embedded.php use get_string for 'home' (although it's only shown as alt text and probably not visible on mobile) <a class="ui-btn-right" data-ajax="false" data-icon="home" href="<?php echo $CFG->wwwroot ?>" data-iconpos="notext">home</a> I also did some testing, not everything, of course: as admin: I was not able to enrol users into course, install language and probably do a lot of other things. But, I guess, administration within mobile theme was not the goal, so I did not test further as a student: When I press 'logout' nothing seem to happen (although I actually log out) My website is located at http://localhost/master/ , after I click the course name the address looks like: http://localhost/master/#/master/course/view.php?id=2 which results in broken icons near activities names in normal browser. In mobile simulator it works ok Navigation button/dropdown looks strange: the 'navigation' option does nothing (and my guess is that it was intended to lead to a page similar to 'Settings'). I would also exclude 'home' from standard navigation because there is another home button on the very top. The title on the button sometimes changes (not sure what was it done for). The items outside of active thread in navigation menu (f.e. My home, Participants, ...) are not shown and are not reachable at all. Actually this looks more like a breadcrumb than navigation. the alerts/messages look like buttons (and have no link of course): <div data-role="button" data-icon="alert" data-theme="d" class="notifysuccess"> some forms (f.e. change password) have very narrow space for labels I wonder if it is possible to hide the 'blocks' button when there are no other blocks on the page when my page is a result of form submission (f.e. in the middle of taking the quiz), the buttons 'blocks' and 'settings' do not work I did not have a chance to test on the real mobile phone, but the simulator shows dropdowns (under navigation, language selector and in activities questions) with really narrow lines. Would be very inconvenient on touch screen device in the standard themes there is a button with course name in the very bottom of the page. It is missing in mymobile theme the course descriptions are missing from the home page (maybe intentionally) hints do not work I backed up a 'features demo' course from qa.moodle.net and restored it into the site with mymobile theme. So I basically tried the different activities as a student. I should tell that a lot do work and look very nice with those big buttons. Some things, unfortunately do not work as expected: file upload screen not themed in 'advanced uploading of files' the Notes edit button does not work checkboxes and radio buttons look strange (see screenshots) quizes use alerts, which are not themed images embedded in html text are not shown in the question texts edit icon is missing in comments (see screenshot) What I want to say that it is a huge task and great work has been done. I'm sure after testing and fixing of the the small issues users will appreciate it very much! Marina
          Hide
          Martin Dougiamas added a comment -

          Marina did a review (see above)

          Show
          Martin Dougiamas added a comment - Marina did a review (see above)
          Hide
          Martin Dougiamas added a comment -

          John,

          How is your time looking to work on this? Can we have a chat sometime? Jabber: moodler@moodle.org

          Show
          Martin Dougiamas added a comment - John, How is your time looking to work on this? Can we have a chat sometime? Jabber: moodler@moodle.org
          Hide
          John Stabinger added a comment -

          I should have some time over the next week or two to do the work. Some of the things have already been corrected, others are intentional (for example, not theming the file upload screen, alerts, etc.).

          Marina, the dropdowns may look odd in a simulator (what simulator are you using BTW?) but they look very good on real devices. It really works best on actual devices.

          Martin, I'll see if I can get in touch with you sometime this week.

          Show
          John Stabinger added a comment - I should have some time over the next week or two to do the work. Some of the things have already been corrected, others are intentional (for example, not theming the file upload screen, alerts, etc.). Marina, the dropdowns may look odd in a simulator (what simulator are you using BTW?) but they look very good on real devices. It really works best on actual devices. Martin, I'll see if I can get in touch with you sometime this week.
          Hide
          John Stabinger added a comment -

          Marina,

          Can you explain further what you mean by: "layout/*.php: the output in tag attributes should be wrapped in p()
          examples:
          <div data-role="page" id="chatpage" data-fullscreen="true" data-title="<?php echo $SITE->shortname ?>">
          <div id="content2" data-role="page" data-title="<?php echo $SITE->shortname ?>">
          layout/embedded.php"

          I'm not following... Also what standard browser were you testing in and in what activities are the screenshots from?

          Show
          John Stabinger added a comment - Marina, Can you explain further what you mean by: "layout/*.php: the output in tag attributes should be wrapped in p() examples: <div data-role="page" id="chatpage" data-fullscreen="true" data-title="<?php echo $SITE->shortname ?>"> <div id="content2" data-role="page" data-title="<?php echo $SITE->shortname ?>"> layout/embedded.php" I'm not following... Also what standard browser were you testing in and in what activities are the screenshots from?
          Hide
          Dongsheng Cai added a comment -

          Hi, John

          p() and his brother s() is used to add quotes to html characters, preventing XSS attack.

          Show
          Dongsheng Cai added a comment - Hi, John p() and his brother s() is used to add quotes to html characters, preventing XSS attack.
          Hide
          Marina Glancy added a comment - - edited

          John,
          if $SITE->shortname contains quotes, the tag will be broken. You should write "<?php echo p($SITE->shortname) ?>"
          in the most places you already use p()

          for screenshots I used Chromium (Chrome) and Ripple plugin for it. And right now I also checked in FireFox and Midori, everywhere the screen looks as on my screenshots.

          Activities are taken from qa.moodle.net. Particular pages are:
          http://qa.moodle.net/mod/lesson/view.php?id=31 (click 'multiple answers' to see checkboxes)
          http://qa.moodle.net/mod/survey/view.php?id=47 (radio buttons)
          http://qa.moodle.net/mod/wiki/comments.php?pageid=1 (comments)

          Show
          Marina Glancy added a comment - - edited John, if $SITE->shortname contains quotes, the tag will be broken. You should write "<?php echo p($SITE->shortname) ?>" in the most places you already use p() for screenshots I used Chromium (Chrome) and Ripple plugin for it. And right now I also checked in FireFox and Midori, everywhere the screen looks as on my screenshots. Activities are taken from qa.moodle.net. Particular pages are: http://qa.moodle.net/mod/lesson/view.php?id=31 (click 'multiple answers' to see checkboxes) http://qa.moodle.net/mod/survey/view.php?id=47 (radio buttons) http://qa.moodle.net/mod/wiki/comments.php?pageid=1 (comments)
          Hide
          Martin Dougiamas added a comment -

          Actually just a small correction, you don't need "echo" ... p() means "print" basically. "p()" is the same as "echo s()"

          Show
          Martin Dougiamas added a comment - Actually just a small correction, you don't need "echo" ... p() means "print" basically. "p()" is the same as "echo s()"
          Hide
          Marina Glancy added a comment -

          Hi, John
          I also tested the theme on my Android phone. Everything looks the same as in simulator except that the select element is fine.

          I made another screenshot where I can't scroll right to see the rest of the big picture (and the same thing with the wide tables) and where I can't see the full text of the notices.

          But on the most screens it looks great!

          Show
          Marina Glancy added a comment - Hi, John I also tested the theme on my Android phone. Everything looks the same as in simulator except that the select element is fine. I made another screenshot where I can't scroll right to see the rest of the big picture (and the same thing with the wide tables) and where I can't see the full text of the notices. But on the most screens it looks great!
          Hide
          John Stabinger added a comment -

          Marina,

          I am just about ready to post up the revised version (which includes JQM beta 2 and many fixes). How should I do that? just update the branch, create new branch and edit the issue? Let me know what the protocol is. Thanks,

          john

          Show
          John Stabinger added a comment - Marina, I am just about ready to post up the revised version (which includes JQM beta 2 and many fixes). How should I do that? just update the branch, create new branch and edit the issue? Let me know what the protocol is. Thanks, john
          Hide
          Marina Glancy added a comment -

          Hi John,
          The issue still has a status 'development in progress'. Making new commit to the same branch is the best idea in this case
          Marina

          Show
          Marina Glancy added a comment - Hi John, The issue still has a status 'development in progress'. Making new commit to the same branch is the best idea in this case Marina
          Hide
          Martin Dougiamas added a comment -

          John,

          Thanks for your work on this ... I'm looking forward to seeing this in core. (In related news I hope that sometime soon we might even switch from YUI to jquery even for all the other core stuff).

          You may not have seen that David Mudrak has done another review of the code, see MDL-28608 (a subtask of this one) for details.

          Show
          Martin Dougiamas added a comment - John, Thanks for your work on this ... I'm looking forward to seeing this in core. (In related news I hope that sometime soon we might even switch from YUI to jquery even for all the other core stuff). You may not have seen that David Mudrak has done another review of the code, see MDL-28608 (a subtask of this one) for details.
          Hide
          John Stabinger added a comment -

          I updated to a new branch (to reflect the feedback from David). The new branch and diff is above. This should fix many of the issues Marina raised.

          John

          Show
          John Stabinger added a comment - I updated to a new branch (to reflect the feedback from David). The new branch and diff is above. This should fix many of the issues Marina raised. John
          Hide
          Martin Dougiamas added a comment - - edited

          I think it's time to start getting this submitted for integration into 2.2

          John, could you please rebase your patch against the latest master and update the fields in this bug, then "submit for integration"?

          Show
          Martin Dougiamas added a comment - - edited I think it's time to start getting this submitted for integration into 2.2 John, could you please rebase your patch against the latest master and update the fields in this bug, then "submit for integration"?
          Hide
          Martin Dougiamas added a comment -

          Ping!

          Show
          Martin Dougiamas added a comment - Ping!
          Hide
          John Stabinger added a comment -

          I'm awake! I will have it ready on Monday or Tues.

          Show
          John Stabinger added a comment - I'm awake! I will have it ready on Monday or Tues.
          Hide
          Dan Poltawski added a comment -

          I was hoping to use this theme on a site which uses an SSO system (currently incompatible with the mobile app due to using this).

          But alas, the site won't be able to use this theme either, as we don't properly redirect people to /login/ for the apache-based SSO system to occur

          Show
          Dan Poltawski added a comment - I was hoping to use this theme on a site which uses an SSO system (currently incompatible with the mobile app due to using this). But alas, the site won't be able to use this theme either, as we don't properly redirect people to /login/ for the apache-based SSO system to occur
          Hide
          John Stabinger added a comment -

          Sorry, running a little behind, we've been without electricity for a few days (snow in October...). I update in the morning with the latest version.

          Show
          John Stabinger added a comment - Sorry, running a little behind, we've been without electricity for a few days (snow in October...). I update in the morning with the latest version.
          Hide
          Mauno Korpelainen added a comment -
          Show
          Mauno Korpelainen added a comment - Great to get this theme to core! Do you have any comments about http://moodle.org/mod/forum/discuss.php?d=188627&parent=821273#p824977 and combining http://jquerymobile.com/blog/2011/10/28/announcing-themeroller-for-mobile-beta/ to mymobile ?
          Hide
          Martin Dougiamas added a comment -

          Mauno, make new bugs for such things.

          We are a couple of days away from 2.2 code freeze and simply must get this into core!

          John, eagerly awaiting the latest code you have, thanks so much.

          Show
          Martin Dougiamas added a comment - Mauno, make new bugs for such things. We are a couple of days away from 2.2 code freeze and simply must get this into core! John, eagerly awaiting the latest code you have, thanks so much.
          Hide
          Mauno Korpelainen added a comment -

          OK - no problem

          I was actually testing mobile themeroller today and it might be best to create a totally new theme based on mymobile and css created by themeroller. Since this is not a bug, just a new feature, I won't create a new bug issue...

          Note that if you test themeroller scripts from GIT the folder where zipped theme files go must have write permissions or downloading themeroller themes won't work. The only major problem was different way to handle urls of images in css files (moodle 2 themes have those custom img paths) but a workaround is to use theme layout file instead of config array for loading custom themeroller css.

          Show
          Mauno Korpelainen added a comment - OK - no problem I was actually testing mobile themeroller today and it might be best to create a totally new theme based on mymobile and css created by themeroller. Since this is not a bug, just a new feature, I won't create a new bug issue... Note that if you test themeroller scripts from GIT the folder where zipped theme files go must have write permissions or downloading themeroller themes won't work. The only major problem was different way to handle urls of images in css files (moodle 2 themes have those custom img paths) but a workaround is to use theme layout file instead of config array for loading custom themeroller css.
          Hide
          Martin Dougiamas added a comment -

          Sam, sorry I missed you this morning. I'm assuming you are starting to look at this for integration this week.

          Show
          Martin Dougiamas added a comment - Sam, sorry I missed you this morning. I'm assuming you are starting to look at this for integration this week.
          Hide
          Sam Hemelryk added a comment -

          Hi Martin, starting the integration review of this issue now

          Show
          Sam Hemelryk added a comment - Hi Martin, starting the integration review of this issue now
          Hide
          Sam Hemelryk added a comment -

          Hi John + everyone,

          First up marvellous effort so far on the theme. It's looking fantastic.

          After talking to Martin about the code it was decided that this is so close that rather than send it back to clean up the minor things that could be cleaned up I would make the required changes as I reviewed it.
          I've spent the past two days reviewing this and making required changes. The theme itself has not changed in appearance at all the changes I have made have been purely behind the scenes although there are a few things that will need to be looked into ASAP.
          Just summarising the main bulk of the changes:

          • Cleaned up a lot of whitespace issues
          • Removed unused functions, variables, strings, and code snippets.
          • Renamed the settings to make them clearers.
          • Moved the renderer in lib.php to renderers.php, renamed it, and changed it's access to the conventional method of acquiring a custom renderer.
          • Cleaned up formatting in the layout files.
          • Added missing phpdocs.
          • Rewrote the layout definitions in config.php to make them maintainable.
          • Both layout files were using SITE->shortname in the title, this would need to be properly cleaned and formatted before using it there. For the time being I have switched it to use PAGE->title like all other themes. (I have no object to this being switched to site short name if cleaned properly).

          I've also added TODO comments throughout the code while reviewing where ever there is something that needs to be looked at.
          Once this gets integrated... and it will.. I will create an issue to look at those, then of course John or anyone else if you could help with that it would be most appreciated.

          Presently I've pushed my changes on a branch to my github:
          The branch in its entirety: https://github.com/samhemelryk/moodle/compare/master...wip-MDL-27622-master
          The changes I made specifically: https://github.com/samhemelryk/moodle/commit/3c0c2bf080a16d71e7338dcaebb4c76eef71b430
          I've asked Apu already if he could please review my changes (it still pays to have all the code checked) so we wait to hear from him

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi John + everyone, First up marvellous effort so far on the theme. It's looking fantastic. After talking to Martin about the code it was decided that this is so close that rather than send it back to clean up the minor things that could be cleaned up I would make the required changes as I reviewed it. I've spent the past two days reviewing this and making required changes. The theme itself has not changed in appearance at all the changes I have made have been purely behind the scenes although there are a few things that will need to be looked into ASAP. Just summarising the main bulk of the changes: Cleaned up a lot of whitespace issues Removed unused functions, variables, strings, and code snippets. Renamed the settings to make them clearers. Moved the renderer in lib.php to renderers.php, renamed it, and changed it's access to the conventional method of acquiring a custom renderer. Cleaned up formatting in the layout files. Added missing phpdocs. Rewrote the layout definitions in config.php to make them maintainable. Both layout files were using SITE->shortname in the title, this would need to be properly cleaned and formatted before using it there. For the time being I have switched it to use PAGE->title like all other themes. (I have no object to this being switched to site short name if cleaned properly). I've also added TODO comments throughout the code while reviewing where ever there is something that needs to be looked at. Once this gets integrated... and it will.. I will create an issue to look at those, then of course John or anyone else if you could help with that it would be most appreciated. Presently I've pushed my changes on a branch to my github: The branch in its entirety: https://github.com/samhemelryk/moodle/compare/master...wip-MDL-27622-master The changes I made specifically: https://github.com/samhemelryk/moodle/commit/3c0c2bf080a16d71e7338dcaebb4c76eef71b430 I've asked Apu already if he could please review my changes (it still pays to have all the code checked) so we wait to hear from him Cheers Sam
          Hide
          Aparup Banerjee added a comment - - edited

          Hi all,
          I've tried to look thru this as well as i can given the haste (noted that theres already been many reviews). I understand that this has to go in anyway and any short comings will have to be fixed eventually anyway:

          • a quick test revealed below errors.
            • Undefined variable: usercol in theme/mymobile/layout/general.php on line 123. $usercol needs to be initialised.
            • the block regions would need to be looked into, adding a block is a fatal act. That said, i understand that this is still OK for browsing.
              ( coding error: "Trying to reference an unknown block region" ...line 632 of /theme/mymobile/renderers.php: call to moodle_page->set_state() , line 3 of unknownfile: call to theme_mymobile_core_renderer->header() ...)
          • also noting that button's text was being truncated on ipad in tablet view when turning from landscape to portrait mode (minor non blocking issue)
          • just noting that theres commented code in some places thats been left around (for further reviewing/performance etc)
          • Testing : Theres lots TODO as Sam mentioned , theres also lots to TEST, we need to define at least some of the tests very soon under testing instructions.

          but all in all, Sam, the changes are (relatively) fine for me! :-D

          Show
          Aparup Banerjee added a comment - - edited Hi all, I've tried to look thru this as well as i can given the haste (noted that theres already been many reviews). I understand that this has to go in anyway and any short comings will have to be fixed eventually anyway: a quick test revealed below errors. Undefined variable: usercol in theme/mymobile/layout/general.php on line 123. $usercol needs to be initialised. the block regions would need to be looked into, adding a block is a fatal act. That said, i understand that this is still OK for browsing. ( coding error: "Trying to reference an unknown block region" ...line 632 of /theme/mymobile/renderers.php: call to moodle_page->set_state() , line 3 of unknownfile: call to theme_mymobile_core_renderer->header() ...) also noting that button's text was being truncated on ipad in tablet view when turning from landscape to portrait mode (minor non blocking issue) just noting that theres commented code in some places thats been left around (for further reviewing/performance etc) Testing : Theres lots TODO as Sam mentioned , theres also lots to TEST, we need to define at least some of the tests very soon under testing instructions. but all in all, Sam, the changes are (relatively) fine for me! :-D
          Hide
          Sam Hemelryk added a comment -

          Thank you everyone involved.... the mymobile theme has just touched down and is within the master branch now ready for 2.2!

          Much kudo's John!

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Thank you everyone involved.... the mymobile theme has just touched down and is within the master branch now ready for 2.2! Much kudo's John! Cheers Sam
          Hide
          Jérôme Mouneyrac added a comment -

          The theme rocks. Thank you.

          Show
          Jérôme Mouneyrac added a comment - The theme rocks. Thank you.
          Hide
          Helen Foster added a comment -

          I'd like to write a QA test for the MyMobile theme. Please could anyone suggest what to include in the test, such as a list of pages to view using a mobile device.

          Show
          Helen Foster added a comment - I'd like to write a QA test for the MyMobile theme. Please could anyone suggest what to include in the test, such as a list of pages to view using a mobile device.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Great job everybody, thanks!!!

          BUT a big Q: What will happen next time one plugin lands to core and also requires jquery? Boom? core plugins must not have own libraries, that does NOT work ok in the long term (see Oauth, see this, see SOAP...).

          That said not matter we are also talking a about jquery, that finally has been injected to core, I hope people won't start using it from their contrib plugins.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Great job everybody, thanks!!! BUT a big Q: What will happen next time one plugin lands to core and also requires jquery? Boom? core plugins must not have own libraries, that does NOT work ok in the long term (see Oauth, see this, see SOAP...). That said not matter we are also talking a about jquery, that finally has been injected to core, I hope people won't start using it from their contrib plugins. Ciao
          Hide
          Mauno Korpelainen added a comment -

          In addition to what Eloy said it would be nice to include the latest jQuery files to lib and also make sure that plugins of moodle can use the LATEST versions of jQuery AND mobile jQuery with the latest bug fixes

          http://jquery.com/

          http://jquerymobile.com/

          If YUI must have a monopoly over moodleland javascripts you could as well switch to cdn jquery

          http://code.jquery.com/jquery-1.7.min.js

          ( or other cdn versions in http://docs.jquery.com/Downloading_jQuery )

          & for mobile jQuery

          http://code.jquery.com/mobile/1.0rc2/jquery.mobile-1.0rc2.min.css

          and

          http://code.jquery.com/mobile/1.0rc2/jquery.mobile-1.0rc2.min.js

          ( or other cdn versions in http://jquerymobile.com/download/ )

          That said it would be great to see more super cool jQuery tools in core moodle and contrib

          Show
          Mauno Korpelainen added a comment - In addition to what Eloy said it would be nice to include the latest jQuery files to lib and also make sure that plugins of moodle can use the LATEST versions of jQuery AND mobile jQuery with the latest bug fixes http://jquery.com/ http://jquerymobile.com/ If YUI must have a monopoly over moodleland javascripts you could as well switch to cdn jquery http://code.jquery.com/jquery-1.7.min.js ( or other cdn versions in http://docs.jquery.com/Downloading_jQuery ) & for mobile jQuery http://code.jquery.com/mobile/1.0rc2/jquery.mobile-1.0rc2.min.css and http://code.jquery.com/mobile/1.0rc2/jquery.mobile-1.0rc2.min.js ( or other cdn versions in http://jquerymobile.com/download/ ) That said it would be great to see more super cool jQuery tools in core moodle and contrib
          Hide
          Mauno Korpelainen added a comment -

          Or you could add a setting to theme settings of mymobile theme that allows administrator to choose from a couple gives options which jQuery to use - the one in local files (separately loaded if not acceptable in core), cdn version (always available if online) or for example local version inside the theme (plugin). That's how for example Geogebra filter chooses location of geogebra applet (different versions may have different functionalities even though most scripts try to keep backwards compatibility - and sometimes a plugin may need to use the older version... )

          Show
          Mauno Korpelainen added a comment - Or you could add a setting to theme settings of mymobile theme that allows administrator to choose from a couple gives options which jQuery to use - the one in local files (separately loaded if not acceptable in core), cdn version (always available if online) or for example local version inside the theme (plugin). That's how for example Geogebra filter chooses location of geogebra applet (different versions may have different functionalities even though most scripts try to keep backwards compatibility - and sometimes a plugin may need to use the older version... )
          Hide
          Eloy Lafuente (stronk7) added a comment -

          hehe, Mauno, I knew the "jquery" and "core" words together in one comment was going to cause a flood here.

          Please, discuss it in forums (I'm getting a lot of good information there in that looong discussion), this is only "tangentially" touching the "jquery-master-query". The important bit here is about have some library included within a core module and what does mean in the long term if the library is potentially used by another plugin/s. I guess some 3rd part plugins also using jquery can break badly suddenly because of this.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - hehe, Mauno, I knew the "jquery" and "core" words together in one comment was going to cause a flood here. Please, discuss it in forums (I'm getting a lot of good information there in that looong discussion), this is only "tangentially" touching the "jquery-master-query". The important bit here is about have some library included within a core module and what does mean in the long term if the library is potentially used by another plugin/s. I guess some 3rd part plugins also using jquery can break badly suddenly because of this. Ciao
          Hide
          Mauno Korpelainen added a comment -

          Well - yes - and - wouldn't it be easier to place all lib files to folder lib - for example lib/jquery ?

          The file theme/mymobile/javascript/jquery-1.6.4.min.js could also be replaced with the latest version jquery-1.7.min.js

          and theme/mymobile/javascript/jquery.mobile-1.0rc2.js and theme/mymobile/style/jmobilerc2.css may have official release pretty soon...

          Personally I have no intention to start long discussions here but I have been testing a few days now a new theme based on mymobile that is using pretty much the same files plus mobile themeroller for customizing given swatches plus a similar swatch-switcher that Sam implemented to Splash theme and I could imagine that all the theme designers around the moodleland might want to create their custom mobile themes once mymobile is officially in core ...

          Show
          Mauno Korpelainen added a comment - Well - yes - and - wouldn't it be easier to place all lib files to folder lib - for example lib/jquery ? The file theme/mymobile/javascript/jquery-1.6.4.min.js could also be replaced with the latest version jquery-1.7.min.js and theme/mymobile/javascript/jquery.mobile-1.0rc2.js and theme/mymobile/style/jmobilerc2.css may have official release pretty soon... Personally I have no intention to start long discussions here but I have been testing a few days now a new theme based on mymobile that is using pretty much the same files plus mobile themeroller for customizing given swatches plus a similar swatch-switcher that Sam implemented to Splash theme and I could imagine that all the theme designers around the moodleland might want to create their custom mobile themes once mymobile is officially in core ...
          Hide
          Mauno Korpelainen added a comment -

          Those loooong discussions in forums don't actually lead to anything because we can't change people's attitudes - they need to change their own attitudes themselves

          What I sincerely hope is that tools like mymobile theme using mobile jQuery would be largely adopted, used and modified further by all moodlers to make moodle have more than one-style, similarlooking faces - for example like http://www.jqmgallery.com/

          Show
          Mauno Korpelainen added a comment - Those loooong discussions in forums don't actually lead to anything because we can't change people's attitudes - they need to change their own attitudes themselves What I sincerely hope is that tools like mymobile theme using mobile jQuery would be largely adopted, used and modified further by all moodlers to make moodle have more than one-style, similarlooking faces - for example like http://www.jqmgallery.com/
          Hide
          Martin Dougiamas added a comment - - edited

          The fact that this theme had to use jquery is unfortunate. The only reason it's going into core is because there is nothing similar in YUI and it's crucial that we have decent mobile functionality in Moodle ASAP.

          Keeping jquery within this module is deliberate because jquery support is not (yet) a core feature of Moodle. Keeping it close to the module ensures the maximum chance that the theme will work as expected without regressions due to jquery changes.

          If anyone starts relying on it for anything other than a derivative theme then they should expect breakage as that CAN NOT BE SUPPORTED.

          If we start including and using jquery for core interfaces then that will be a major policy change, and then we'll of course put it in a core location.

          Show
          Martin Dougiamas added a comment - - edited The fact that this theme had to use jquery is unfortunate. The only reason it's going into core is because there is nothing similar in YUI and it's crucial that we have decent mobile functionality in Moodle ASAP. Keeping jquery within this module is deliberate because jquery support is not (yet) a core feature of Moodle. Keeping it close to the module ensures the maximum chance that the theme will work as expected without regressions due to jquery changes. If anyone starts relying on it for anything other than a derivative theme then they should expect breakage as that CAN NOT BE SUPPORTED. If we start including and using jquery for core interfaces then that will be a major policy change, and then we'll of course put it in a core location.
          Hide
          Andrew Davis added a comment -

          Im using this theme on a laptop which looks a bit odd but its still beautiful Not sure if that is the source of some of these problems.

          I'm logged in as admin.

          On the site front page under available courses if I click on a course I'm enrolled in I get a loading icon in the middle of the screen and the button for the course changes color from whitish to tan but nothing happens. The loading icon isn't animated although it looks like it should be. Not sure if that's significant.

          I just toggled from 2 column, to 1 column and back to 2 column and now clicking the course name seems to work. The loading icon still doesn't animate but the course does load after a second or two.

          The site description displays as a bit empty box with a plus sign in it. Clicking the plus sign displays the site description. Could it display a label to tell you what is in the collapsed box the same way calendar does?

          On the messages screen the search button doesn't seem to do anything. Going to recent conversations looks like it is doing something but that something is obscured by a big blue box. Screenshot attached.

          Apologies if I'm wasting your time by doing this on a laptop.

          Show
          Andrew Davis added a comment - Im using this theme on a laptop which looks a bit odd but its still beautiful Not sure if that is the source of some of these problems. I'm logged in as admin. On the site front page under available courses if I click on a course I'm enrolled in I get a loading icon in the middle of the screen and the button for the course changes color from whitish to tan but nothing happens. The loading icon isn't animated although it looks like it should be. Not sure if that's significant. I just toggled from 2 column, to 1 column and back to 2 column and now clicking the course name seems to work. The loading icon still doesn't animate but the course does load after a second or two. The site description displays as a bit empty box with a plus sign in it. Clicking the plus sign displays the site description. Could it display a label to tell you what is in the collapsed box the same way calendar does? On the messages screen the search button doesn't seem to do anything. Going to recent conversations looks like it is doing something but that something is obscured by a big blue box. Screenshot attached. Apologies if I'm wasting your time by doing this on a laptop.
          Hide
          Mary Evans added a comment - - edited

          Andrew,

          I trust you will be testing this using a mobile device and not just a laptop as I reckon this is a waste of time.

          As far as I am aware what happens is that the main site would use a normal theme say Afterburner since it's the first on the list of standard Moodle themes. MyMobile theme would then be selected as a theme for a Mobile device and a Tablet (I presume).

          Settings for the types of devices allowed are then set in Theme settings page. This is all done using a normal standard theme on a computer. When everything is set up then testing can start using a mobile device.

          Once logged into the site from a mobile the MyMobile theme kicks in and away you go. The mobile device is not meant as an administrators toy, neither is it meant for a laptop,..at least I would not have thought so.

          Cheers
          Mary

          Show
          Mary Evans added a comment - - edited Andrew, I trust you will be testing this using a mobile device and not just a laptop as I reckon this is a waste of time. As far as I am aware what happens is that the main site would use a normal theme say Afterburner since it's the first on the list of standard Moodle themes. MyMobile theme would then be selected as a theme for a Mobile device and a Tablet (I presume). Settings for the types of devices allowed are then set in Theme settings page. This is all done using a normal standard theme on a computer. When everything is set up then testing can start using a mobile device. Once logged into the site from a mobile the MyMobile theme kicks in and away you go. The mobile device is not meant as an administrators toy, neither is it meant for a laptop,..at least I would not have thought so. Cheers Mary
          Hide
          Andrew Davis added a comment -

          Theoretically the theme should perform the same on a laptop as on a mobile device. Just bigger...

          All the same Ive stopped testing to make this available for someone in HQ. If Im correct in my understanding of how this works they should be able to view their test site on their phones via their myname.moodle.local addresses. Im not familiar with the process of setting up my laptop on a public LAN to allow my phone to browse it which probably means it will take me longer than is worthwhile if someone else has a set up ready to go.

          For anyone who does take this on apparently admin type functions are known to be very broken at the moment so don't log in as admin.

          Show
          Andrew Davis added a comment - Theoretically the theme should perform the same on a laptop as on a mobile device. Just bigger... All the same Ive stopped testing to make this available for someone in HQ. If Im correct in my understanding of how this works they should be able to view their test site on their phones via their myname.moodle.local addresses. Im not familiar with the process of setting up my laptop on a public LAN to allow my phone to browse it which probably means it will take me longer than is worthwhile if someone else has a set up ready to go. For anyone who does take this on apparently admin type functions are known to be very broken at the moment so don't log in as admin.
          Hide
          Mauno Korpelainen added a comment -

          Andrew,

          mymobile should work normally in mobile, laptops and desktop devices - and mobile jQuery supports most current browsers http://jquerymobile.com/gbs/

          Which browser / OS was it? Is it the latest version of mymobile? Does the laptop have some 3rd party browser plugins etc?

          Show
          Mauno Korpelainen added a comment - Andrew, mymobile should work normally in mobile, laptops and desktop devices - and mobile jQuery supports most current browsers http://jquerymobile.com/gbs/ Which browser / OS was it? Is it the latest version of mymobile? Does the laptop have some 3rd party browser plugins etc?
          Hide
          Andrew Davis added a comment -

          Firefox 7.01, Ubuntu 11.04. Its the version of mymobile that is currently on the Moodle integration server. I believe Firebug is the only plugin I have installed. Hope thats helpful.

          Show
          Andrew Davis added a comment - Firefox 7.01, Ubuntu 11.04. Its the version of mymobile that is currently on the Moodle integration server. I believe Firebug is the only plugin I have installed. Hope thats helpful.
          Hide
          Martin Dougiamas added a comment -

          Closing this as it's not a normal test. Goes through MDLQA now.

          Show
          Martin Dougiamas added a comment - Closing this as it's not a normal test. Goes through MDLQA now.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          I've added one extra commit related to this, addign the mymobile to the list of core themes in lib/pluginlib.php (so it will be reported as standard one, and not extension).

          FYI, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - I've added one extra commit related to this, addign the mymobile to the list of core themes in lib/pluginlib.php (so it will be reported as standard one, and not extension). FYI, ciao
          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:
              5 Vote for this issue
              Watchers:
              13 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: