Moodle
  1. Moodle
  2. MDL-27588

formal_white umbrella for a mixture of improvements

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor 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:
    • Rank:
      17518

      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.

        Issue Links

          Activity

          Hide
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          Sam Hemelryk added a comment -

          Thanks Daniele this has been integrated now.
          Cheers
          Sam

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

          tty

          Show
          Daniele Cordella added a comment - tty
          Hide
          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
          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
          Eloy Lafuente (stronk7) added a comment -

          This is now upstream, yay! Many thanks!

          Show
          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: