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

formal_white umbrella for a mixture of improvements

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0.3
    • Fix Version/s: 2.0.4
    • Component/s: Themes
    • Labels:
      None
    • Testing Instructions:
      Hide

      see MDL-26934 and MDL-27474 (I apologise if this interation review is not well filled. I am learning and I am still having troubles)

      Show
      see MDL-26934 and MDL-27474 (I apologise if this interation review is not well filled. I am learning and I am still having troubles)
    • Affected Branches:
      MOODLE_20_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE
    • Pull from Repository:

      Description

      This is what changed since last push:
      -> MDL-26934 has been fixed
      -> MDL-27474 has been fixed
      -> one more option to have blocks column with different background colours
      -> one more option to use custom font size
      -> version.php, db/install.php and db/upgrade.php to handle upgrades
      -> increased the font size in the table with plugin list shown at upgrade time
      -> minor fixes in IE7 when images from remote web sites are shown in the header
      -> conformed the dock background colour to the block background colour to avoid unpleasant color combinations
      -> conformed a.link, a.visited and a.active colours in tabs
      -> conformed a.link, a.visited and a.active colours in the navigation block
      -> deleted a awful box border from enrolment page
      -> right aligned commands for resources and activities in the course page
      -> increased the width of field item title column in mform
      -> increased the padding-top to the help popup box messages
      -> user menu restyled to fix some minor tweaks in IE7
      -> minor repair to tabs to improve its look
      -> restyling of the docked item panel header to look like all the other blocks
      -> reduced the distance between elements in headermenu to look better even when only headings are shown in the header
      -> links wherever and always black
      -> the "search courses" field in the navigation bar does not force extra height
      -> some unpleasant generalbox border duplicating some other border have been removed
      -> some unpleasant margin/padding in notices and mtables has been removed

      even more: people is complaining about MDL-25775. I tested it on my local computer and it worked since last push. Please testers, make one more independent check and tell the last word.

      known issues:
      edit courses: table in the main part of the page has a strange alignment. It depends from: MDL-26759
      manage blocks: table in the main part of the page has a strange alignment. It depends from: MDL-26760

      A note: I am not sure I gave right permissions to files! Sorry.
      Sam, I believe I fixed permissions.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            lazydaisy Mary Evans added a comment -

            Hi Daniele,

            There are reports on the forum of the CSS Custom Settings not taking effect.
            On further inspection a function in lib.php seems to be missing. Also the csspostprocess is not set as other theme which have custom settings.

            If these have already been fixed, which version of Moodle 2.0.x is the fix in so I can test it?

            Thanks
            Mary

            Show
            lazydaisy Mary Evans added a comment - Hi Daniele, There are reports on the forum of the CSS Custom Settings not taking effect. On further inspection a function in lib.php seems to be missing. Also the csspostprocess is not set as other theme which have custom settings. If these have already been fixed, which version of Moodle 2.0.x is the fix in so I can test it? Thanks Mary
            Hide
            daniss Daniele Cordella added a comment -

            Ciao Mary.
            This is the release fixing what you describe.
            I wrote, among what this improvement changes, MDL-27474.
            Thanks to you for spotting this out!
            Daniele

            Show
            daniss Daniele Cordella added a comment - Ciao Mary. This is the release fixing what you describe. I wrote, among what this improvement changes, MDL-27474 . Thanks to you for spotting this out! Daniele
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Hi Daniele,

            I've been looking over this for the past while, I have to admit I was slightly confused to begin with but I think I sorted it out.
            The details for the work you've done arn't quite right, I'll edit the issue after this so you can see what they should be.

            As for the changes I spotted the following things while reviewing it:

            • Unneeded upgrade.php file
            • version.php had $module->requires
            • The region-side-post and region-side-pre strings shou;dn't be removed - they are needed for the block settings pages.
            • Default for $fontsizereference shoud be '13' not '13px' because formal_white_set_fontsizereference adds the px.
            • You only have these changes on MOODLE_20_STABLE they need to be ported to the master branch as well.
            • All your commit messages should start with the tracker issue number (MDL-27588 in this case) as a general rule of thumb we won't integrate changes if the commit messages don't contain the issue number as we need it to relate changes back to issue numbers.
            • There were still several files that had incorrect permissions.

            Most of these are trivial and the rest you'll learn to avoid as part of learning to use git.
            I wrote the following doc: http://docs.moodle.org/en/User:Sam_Hemelryk/My_Moodle_Git_workflow which you may find useful as a reference and there are links to pages I found useful while learning git down the bottom.

            As for what to do with it this week.... I've made the required changes and pushed things to my github account this week so you can see what they look like:

            Git repo: git://github.com/samhemelryk/moodle.git

            master branch: wip-MDL-27588-master
            master diff: https://github.com/samhemelryk/moodle/compare/master...wip-MDL-27588-master

            MOODLE_20_STABLE branch: wip-MDL-27588-MOODLE_20_STABLE
            MOODLE_20_STABLE diff: https://github.com/samhemelryk/moodle/compare/MOODLE_20_STABLE...wip-MDL-27588-MOODLE_20_STABLE

            I'll talk to Eloy today when he comes online and see what he thinks - if you could please have a look at those and check that I havn't missed any of the changes you have made.

            Cheers
            Sam

            Show
            samhemelryk Sam Hemelryk added a comment - Hi Daniele, I've been looking over this for the past while, I have to admit I was slightly confused to begin with but I think I sorted it out. The details for the work you've done arn't quite right, I'll edit the issue after this so you can see what they should be. As for the changes I spotted the following things while reviewing it: Unneeded upgrade.php file version.php had $module->requires The region-side-post and region-side-pre strings shou;dn't be removed - they are needed for the block settings pages. Default for $fontsizereference shoud be '13' not '13px' because formal_white_set_fontsizereference adds the px. You only have these changes on MOODLE_20_STABLE they need to be ported to the master branch as well. All your commit messages should start with the tracker issue number ( MDL-27588 in this case) as a general rule of thumb we won't integrate changes if the commit messages don't contain the issue number as we need it to relate changes back to issue numbers. There were still several files that had incorrect permissions. Most of these are trivial and the rest you'll learn to avoid as part of learning to use git. I wrote the following doc: http://docs.moodle.org/en/User:Sam_Hemelryk/My_Moodle_Git_workflow which you may find useful as a reference and there are links to pages I found useful while learning git down the bottom. As for what to do with it this week.... I've made the required changes and pushed things to my github account this week so you can see what they look like: Git repo: git://github.com/samhemelryk/moodle.git master branch: wip- MDL-27588 -master master diff: https://github.com/samhemelryk/moodle/compare/master...wip-MDL-27588-master MOODLE_20_STABLE branch: wip- MDL-27588 -MOODLE_20_STABLE MOODLE_20_STABLE diff: https://github.com/samhemelryk/moodle/compare/MOODLE_20_STABLE...wip-MDL-27588-MOODLE_20_STABLE I'll talk to Eloy today when he comes online and see what he thinks - if you could please have a look at those and check that I havn't missed any of the changes you have made. Cheers Sam
            Hide
            daniss Daniele Cordella added a comment -

            Thanks Sam. I am slooooooowly learning with your help and mainly with the effort of the great Eloy. My morning (it is 10:45 now here where I am) is crowded of professional duty here and I will be back to "passion" only in the afternoon/evening. In the meanwhile: thanks for your comprehension! I will try to check all you did. I apologise for all the things you found wrong like the address of my github repo, the missing diff URL and so forth. Thanks again. I hope my work and you corrections may be useful to moodle community.

            Show
            daniss Daniele Cordella added a comment - Thanks Sam. I am slooooooowly learning with your help and mainly with the effort of the great Eloy. My morning (it is 10:45 now here where I am) is crowded of professional duty here and I will be back to "passion" only in the afternoon/evening. In the meanwhile: thanks for your comprehension! I will try to check all you did. I apologise for all the things you found wrong like the address of my github repo, the missing diff URL and so forth. Thanks again. I hope my work and you corrections may be useful to moodle community.
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Hi Daniele that is OK.

            I will reject it this week just so that you can have a look at the changes I've made, make sure I haven't missed anything and then fix up this issue.
            What we are looking for is the changes up on your github account for both the Moodle 20 stable and master branches and this issue updated with information.
            I've talked this through with Eloy now and he says you can ask him if you want any help/instruction with that

            Cheers
            Sam

            Show
            samhemelryk Sam Hemelryk added a comment - Hi Daniele that is OK. I will reject it this week just so that you can have a look at the changes I've made, make sure I haven't missed anything and then fix up this issue. What we are looking for is the changes up on your github account for both the Moodle 20 stable and master branches and this issue updated with information. I've talked this through with Eloy now and he says you can ask him if you want any help/instruction with that Cheers Sam
            Hide
            daniss Daniele Cordella added a comment -

            @Sam: I am supposed to join my office in about 40 minutes. If you will be online I will contact you to ask for details (just to avoid to always push Eloy Eloy and Eloy!)

            Show
            daniss Daniele Cordella added a comment - @Sam: I am supposed to join my office in about 40 minutes. If you will be online I will contact you to ask for details (just to avoid to always push Eloy Eloy and Eloy!)
            Hide
            daniss Daniele Cordella added a comment - - edited

            Unneeded upgrade.php file
            > I agree. I added it for future reference but, I agree, it is now useless.

            version.php had $module->requires
            > I apologise but I can not understand this issue. You stated it in http://docs.moodle.org/en/Development:Themes_2.0_adding_upgrade_code.
            Citation from your page: "The version property is required and the rest are optional however it is best practice to put them in as well."

            The region-side-post and region-side-pre strings shouldn't be removed - they are needed for the block settings pages.
            > wow, this is an info I was still missing. Thanks!

            Default for $fontsizereference should be '13' not '13px' because formal_white_set_fontsizereference adds the px.
            > you are right. My fault. Sorry.

            You only have these changes on MOODLE_20_STABLE they need to be ported to the master branch as well.
            > I would like to know how to do it. I agree, I will get it. Please be patient.

            All your commit messages should start with the tracker issue number (MDL-27588 in this case) as a general rule of thumb we won't integrate changes if the commit messages don't contain the issue number as we need it to relate changes back to issue numbers.
            > ok, one more info to learn. Thanks!

            There were still several files that had incorrect permissions.
            > this sounds strange to me.

            I did this:
            cd /path/to/my/moodle/master/folder
            git co formal_white_umbrella
            find . -type f -exec chmod 644 {} \;
            find . -type d -exec chmod 755 {} \;
            git commit -m 'permissions corrected'
            git push origin formal_white_umbrella

            Where was I wrong?
            Thanks in advance.

            Am I supposed to do more?
            Am I allowed to "Submit for integration" one more time?

            Show
            daniss Daniele Cordella added a comment - - edited Unneeded upgrade.php file > I agree. I added it for future reference but, I agree, it is now useless. version.php had $module->requires > I apologise but I can not understand this issue. You stated it in http://docs.moodle.org/en/Development:Themes_2.0_adding_upgrade_code . Citation from your page: "The version property is required and the rest are optional however it is best practice to put them in as well." The region-side-post and region-side-pre strings shouldn't be removed - they are needed for the block settings pages. > wow, this is an info I was still missing. Thanks! Default for $fontsizereference should be '13' not '13px' because formal_white_set_fontsizereference adds the px. > you are right. My fault. Sorry. You only have these changes on MOODLE_20_STABLE they need to be ported to the master branch as well. > I would like to know how to do it. I agree, I will get it. Please be patient. All your commit messages should start with the tracker issue number ( MDL-27588 in this case) as a general rule of thumb we won't integrate changes if the commit messages don't contain the issue number as we need it to relate changes back to issue numbers. > ok, one more info to learn. Thanks! There were still several files that had incorrect permissions. > this sounds strange to me. I did this: cd /path/to/my/moodle/master/folder git co formal_white_umbrella find . -type f -exec chmod 644 {} \; find . -type d -exec chmod 755 {} \; git commit -m 'permissions corrected' git push origin formal_white_umbrella Where was I wrong? Thanks in advance. Am I supposed to do more? Am I allowed to "Submit for integration" one more time?
            Hide
            daniss Daniele Cordella added a comment -

            Ciao Sam
            maybe you found some permission still not correct because I didn't point my last push but one of the previous.
            This is what I did: https://github.com/kordan/moodle/commits/formal_white_umbrella
            I apologise for the disorder of this push.

            Show
            daniss Daniele Cordella added a comment - Ciao Sam maybe you found some permission still not correct because I didn't point my last push but one of the previous. This is what I did: https://github.com/kordan/moodle/commits/formal_white_umbrella I apologise for the disorder of this push.
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Thanks Daniele this has been integrated now.
            Cheers
            Sam

            Show
            samhemelryk Sam Hemelryk added a comment - Thanks Daniele this has been integrated now. Cheers Sam
            Hide
            daniss Daniele Cordella added a comment -

            tty

            Show
            daniss Daniele Cordella added a comment - tty
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            I've been playing with this all the day and haven't found any annoyance, so I'd consider this passed.

            Only one note, that surely needs (to be created) new issue. When installing it I got this PHP Notice, doesn't seem important enough to reject this issue but needs fixing. It happened both in the 2.0 and 2.1 installation:

            -->theme_formal_white
            PHP Notice:  Undefined property: stdClass::$blockcolumnbgc in /Users/Shared/Jenkins/Home/moodle_integration_git/theme/formal_white/db/install.php on line 62
            PHP Stack trace:
            PHP   1. {main}() /Users/Shared/Jenkins/Home/moodle_integration_git/admin/cli/install.php:0
            PHP   2. install_cli_database() /Users/Shared/Jenkins/Home/moodle_integration_git/admin/cli/install.php:648
            PHP   3. upgrade_noncore() /Users/Shared/Jenkins/Home/moodle_integration_git/lib/installlib.php:559
            PHP   4. upgrade_plugins() /Users/Shared/Jenkins/Home/moodle_integration_git/lib/upgradelib.php:1464
            PHP   5. xmldb_theme_formal_white_install() /Users/Shared/Jenkins/Home/moodle_integration_git/lib/upgradelib.php:353
             
            Notice: Undefined property: stdClass::$blockcolumnbgc in /Users/Shared/Jenkins/Home/moodle_integration_git/theme/formal_white/db/install.php on line 62
             
            Call Stack:
                0.0015     957712   1. {main}() /Users/Shared/Jenkins/Home/moodle_integration_git/admin/cli/install.php:0
                0.4154   32759296   2. install_cli_database() /Users/Shared/Jenkins/Home/moodle_integration_git/admin/cli/install.php:648
                3.6973   37286888   3. upgrade_noncore() /Users/Shared/Jenkins/Home/moodle_integration_git/lib/installlib.php:559
               10.7483   56404520   4. upgrade_plugins() /Users/Shared/Jenkins/Home/moodle_integration_git/lib/upgradelib.php:1464
               10.7538   56420504   5. xmldb_theme_formal_white_install() /Users/Shared/Jenkins/Home/moodle_integration_git/lib/upgradelib.php:353

            detected using the cli installer (I think the web one use to hide those messages).

            Ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - I've been playing with this all the day and haven't found any annoyance, so I'd consider this passed. Only one note, that surely needs (to be created) new issue. When installing it I got this PHP Notice, doesn't seem important enough to reject this issue but needs fixing. It happened both in the 2.0 and 2.1 installation: -->theme_formal_white PHP Notice: Undefined property: stdClass::$blockcolumnbgc in /Users/Shared/Jenkins/Home/moodle_integration_git/theme/formal_white/db/install.php on line 62 PHP Stack trace: PHP 1. {main}() /Users/Shared/Jenkins/Home/moodle_integration_git/admin/cli/install.php:0 PHP 2. install_cli_database() /Users/Shared/Jenkins/Home/moodle_integration_git/admin/cli/install.php:648 PHP 3. upgrade_noncore() /Users/Shared/Jenkins/Home/moodle_integration_git/lib/installlib.php:559 PHP 4. upgrade_plugins() /Users/Shared/Jenkins/Home/moodle_integration_git/lib/upgradelib.php:1464 PHP 5. xmldb_theme_formal_white_install() /Users/Shared/Jenkins/Home/moodle_integration_git/lib/upgradelib.php:353   Notice: Undefined property: stdClass::$blockcolumnbgc in /Users/Shared/Jenkins/Home/moodle_integration_git/theme/formal_white/db/install.php on line 62   Call Stack: 0.0015 957712 1. {main}() /Users/Shared/Jenkins/Home/moodle_integration_git/admin/cli/install.php:0 0.4154 32759296 2. install_cli_database() /Users/Shared/Jenkins/Home/moodle_integration_git/admin/cli/install.php:648 3.6973 37286888 3. upgrade_noncore() /Users/Shared/Jenkins/Home/moodle_integration_git/lib/installlib.php:559 10.7483 56404520 4. upgrade_plugins() /Users/Shared/Jenkins/Home/moodle_integration_git/lib/upgradelib.php:1464 10.7538 56420504 5. xmldb_theme_formal_white_install() /Users/Shared/Jenkins/Home/moodle_integration_git/lib/upgradelib.php:353 detected using the cli installer (I think the web one use to hide those messages). Ciao
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            This is now upstream, yay! Many thanks!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - This is now upstream, yay! Many thanks!

              People

              • Votes:
                3 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  1/Aug/11