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

          Attachments

            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