Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-30600

MyMobile theme throws renderer error in mod choice activity

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.2, 2.3
    • Fix Version/s: 2.2.1
    • Component/s: Themes
    • Labels:

      Description

      One of the renderers used in the mymobile theme to display the mod choice activity is outdated, causing a coding error.

        Gliffy Diagrams

          Activity

          Hide
          salvetore Michael de Raadt added a comment -

          I've put this on the STABLE backlog, but please keep working on it.

          Show
          salvetore Michael de Raadt added a comment - I've put this on the STABLE backlog, but please keep working on it.
          Hide
          epsd John Stabinger added a comment -

          To test, visit a choice activity while using the MyMobile theme and confirm that no coding errors are reported.

          Show
          epsd John Stabinger added a comment - To test, visit a choice activity while using the MyMobile theme and confirm that no coding errors are reported.
          Hide
          rwijaya Rossiani Wijaya added a comment -

          Hi John,

          Thank you for supplying patch to improve Mobile theme.

          However I could not apply the patch due to trailing whitespaces.

          Could you update and resubmit the patch?

          Thanks

          Rosie.

          Show
          rwijaya Rossiani Wijaya added a comment - Hi John, Thank you for supplying patch to improve Mobile theme. However I could not apply the patch due to trailing whitespaces. Could you update and resubmit the patch? Thanks Rosie.
          Hide
          epsd John Stabinger added a comment -

          Could you be more specific? I just copied the code (exactly) from the renderer of the mod choice activity... Is the problem there?

          Show
          epsd John Stabinger added a comment - Could you be more specific? I just copied the code (exactly) from the renderer of the mod choice activity... Is the problem there?
          Hide
          rwijaya Rossiani Wijaya added a comment - - edited

          When I tried to apply your patch to my local machine it showed the following error message:

          @desktop$ git apply ~/patches_git/review/MDL-30600_2.diff
          /home/patches_git/review/MDL-30600_2.diff:9: trailing whitespace.
           
          /home/patches_git/review/MDL-30600_2.diff:10: trailing whitespace.
           
          /home/patches_git/review/MDL-30600_2.diff:27: trailing whitespace.
           
          /home/patches_git/review/MDL-30600_2.diff:35: trailing whitespace.
              
          /home/patches_git/review/MDL-30600_2.diff:194: trailing whitespace.
              
          warning: 5 lines add whitespace errors.
           

          Show
          rwijaya Rossiani Wijaya added a comment - - edited When I tried to apply your patch to my local machine it showed the following error message: @desktop$ git apply ~/patches_git/review/MDL-30600_2.diff /home/patches_git/review/MDL-30600_2.diff:9: trailing whitespace.   /home/patches_git/review/MDL-30600_2.diff:10: trailing whitespace.   /home/patches_git/review/MDL-30600_2.diff:27: trailing whitespace.   /home/patches_git/review/MDL-30600_2.diff:35: trailing whitespace. /home/patches_git/review/MDL-30600_2.diff:194: trailing whitespace. warning: 5 lines add whitespace errors.  
          Hide
          epsd John Stabinger added a comment -

          I'm sorry, but I'm just not seeing this. Could you provide line numbers or something? Again, the code was just copied from an existing moodle core module.

          Show
          epsd John Stabinger added a comment - I'm sorry, but I'm just not seeing this. Could you provide line numbers or something? Again, the code was just copied from an existing moodle core module.
          Hide
          rwijaya Rossiani Wijaya added a comment -

          Hi John,

          I fixed the trailing whitespaces error on the patch.

          Submitting this for integration review.

          Show
          rwijaya Rossiani Wijaya added a comment - Hi John, I fixed the trailing whitespaces error on the patch. Submitting this for integration review.
          Hide
          stronk7 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
          stronk7 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
          samhemelryk Sam Hemelryk added a comment -

          Hi Rosie,

          Is this change needed in the MOODLE_22_STABLE branch by chance?
          I am thinking that given master and MOODLE_22_STABLE have not diverged yet that it is, but could you please check.

          Cheers
          Sam

          Show
          samhemelryk Sam Hemelryk added a comment - Hi Rosie, Is this change needed in the MOODLE_22_STABLE branch by chance? I am thinking that given master and MOODLE_22_STABLE have not diverged yet that it is, but could you please check. Cheers Sam
          Hide
          rwijaya Rossiani Wijaya added a comment -

          Hi Sam,

          Yes, the change is needed for MOODLE_22_STABLE.

          Thank you.

          Show
          rwijaya Rossiani Wijaya added a comment - Hi Sam, Yes, the change is needed for MOODLE_22_STABLE. Thank you.
          Hide
          samhemelryk Sam Hemelryk added a comment -

          Hi guys,

          I'm sending this back presently.
          The level of duplication here is not great and can be avoided.
          At a glance theme_mymobile_mod_choice_renderer extending plugin_renderer_base isn't a great solution, by the looks if it extends mod_choice_renderer instead this sort of duplication can be avoided. (calls to this would default to the parent if there was nothing overridden).

          Show
          samhemelryk Sam Hemelryk added a comment - Hi guys, I'm sending this back presently. The level of duplication here is not great and can be avoided. At a glance theme_mymobile_mod_choice_renderer extending plugin_renderer_base isn't a great solution, by the looks if it extends mod_choice_renderer instead this sort of duplication can be avoided. (calls to this would default to the parent if there was nothing overridden).
          Hide
          rwijaya Rossiani Wijaya added a comment -

          Hi Sam,

          I updated the patch according to you suggestion.

          Re-submitting for integration review.

          Show
          rwijaya Rossiani Wijaya added a comment - Hi Sam, I updated the patch according to you suggestion. Re-submitting for integration review.
          Hide
          nebgor Aparup Banerjee added a comment -

          grabbing this as requested by Rosie/Sam

          Show
          nebgor Aparup Banerjee added a comment - grabbing this as requested by Rosie/Sam
          Hide
          nebgor Aparup Banerjee added a comment -

          Thanks, thats been integrated into master and 2.2.x

          Tester, please report any other notices too (it would be helpful towards getting all these sorted in the mymobile theme)

          Show
          nebgor Aparup Banerjee added a comment - Thanks, thats been integrated into master and 2.2.x Tester, please report any other notices too (it would be helpful towards getting all these sorted in the mymobile theme)
          Hide
          nebgor Aparup Banerjee added a comment -

          works for me - no notices under choice in my iphone + mymobile theme

          Show
          nebgor Aparup Banerjee added a comment - works for me - no notices under choice in my iphone + mymobile theme
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          The git and cvs repositories are happy receiving your very first contribution to Moodle for 2012. Happy new year!

          Closing, ciao

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - The git and cvs repositories are happy receiving your very first contribution to Moodle for 2012. Happy new year! Closing, ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved:
                Fix Release Date:
                9/Jan/12