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

Padding at top of /course/manage.php

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.5, 2.6
    • Fix Version/s: 2.5.1
    • Component/s: Course, Themes
    • Labels:
    • Testing Instructions:
      Hide

      To be tested only for standard themes (bootstrap ones will be fixed by MDL-40485)

      • Open /course/manage.php
      • Confirm that the page appears correctly - specifically
        • Buttons (see screenshot for some examples of what to look out for)
        • Page padding - the header should be in the same place as other pages in Moodle
      Show
      To be tested only for standard themes (bootstrap ones will be fixed by MDL-40485 ) Open /course/manage.php Confirm that the page appears correctly - specifically Buttons (see screenshot for some examples of what to look out for) Page padding - the header should be in the same place as other pages in Moodle
    • Affected Branches:
      MOODLE_25_STABLE, MOODLE_26_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE
    • Pull Master Branch:

      Description

      There is padding on the body of /course/manage.php which causes it to appear incorrectly.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            lazydaisy Mary Evans added a comment - - edited

            It looks to me that there maybe some another selector missing from #page-admin-course-manage like .singlebutton perhaps? Then this would make sense!

             
            .path-admin-roles .buttons .singlebutton,
            #page-admin-course-manage .buttons .singlebutton {
                display: inline;
                padding: 5px;
            }
            

            I have seen this happen before in themes, and is commonplace. This is why Bootstap works better as the class is set and has nothing to do with location. So .buttons .singlebutton would have been enough to style it. Where the selector classes could be ...

            .buttons {
                display:inline;
            } 
            .singlebutton {
                padding : 5px;
            } 
            
            

            Show
            lazydaisy Mary Evans added a comment - - edited It looks to me that there maybe some another selector missing from #page-admin-course-manage like .singlebutton perhaps? Then this would make sense!   .path-admin-roles .buttons .singlebutton, #page-admin-course-manage .buttons .singlebutton { display: inline; padding: 5px; } I have seen this happen before in themes, and is commonplace. This is why Bootstap works better as the class is set and has nothing to do with location. So .buttons .singlebutton would have been enough to style it. Where the selector classes could be ... .buttons { display:inline; } .singlebutton { padding : 5px; }
            Hide
            lazydaisy Mary Evans added a comment -

            Just looking at this page in afterburner and see that the CSS needs to read...

            .path-admin-roles .buttons .singlebutton,
            #page-admin-course-manage .buttons {
                display: inline;
                padding: 5px;
            }

            Show
            lazydaisy Mary Evans added a comment - Just looking at this page in afterburner and see that the CSS needs to read... .path-admin-roles .buttons .singlebutton, #page-admin-course-manage .buttons { display: inline; padding: 5px; }
            Hide
            lazydaisy Mary Evans added a comment -

            @Andrew, this is a graphical representation of what the correct CSS should be and how it should look.

            Show
            lazydaisy Mary Evans added a comment - @Andrew, this is a graphical representation of what the correct CSS should be and how it should look.
            Hide
            lazydaisy Mary Evans added a comment -

            Please amend and then submit for integration.

            Show
            lazydaisy Mary Evans added a comment - Please amend and then submit for integration.
            Hide
            dobedobedoh Andrew Nicols added a comment -

            Updated as per Mary Evans suggestions.

            Show
            dobedobedoh Andrew Nicols added a comment - Updated as per Mary Evans suggestions.
            Hide
            lazydaisy Mary Evans added a comment -

            Andrew, did you actually push the changes to origin as they are not visible in your current commits.

            Show
            lazydaisy Mary Evans added a comment - Andrew, did you actually push the changes to origin as they are not visible in your current commits.
            Hide
            dobedobedoh Andrew Nicols added a comment -

            Mary Evans I did - I've just adjusted the diff URL a little so that the diff should be reasonable now

            Show
            dobedobedoh Andrew Nicols added a comment - Mary Evans I did - I've just adjusted the diff URL a little so that the diff should be reasonable now
            Hide
            lazydaisy Mary Evans added a comment - - edited

            Sorry to be a pain, but this is all you need to change in the original css...

            .path-admin-roles .buttons .singlebutton,
            #page-admin-course-manage .buttons

            { display: inline; padding: 5px; }
            Show
            lazydaisy Mary Evans added a comment - - edited Sorry to be a pain, but this is all you need to change in the original css... .path-admin-roles .buttons .singlebutton, #page-admin-course-manage .buttons { display: inline; padding: 5px; }
            Hide
            lazydaisy Mary Evans added a comment -

            M25 branch is OK only Master branch is showing old commit!

            Show
            lazydaisy Mary Evans added a comment - M25 branch is OK only Master branch is showing old commit!
            Hide
            dobedobedoh Andrew Nicols added a comment -

            Bah - thanks Mary. I must have opened the m25 link twice when I opened into new tabs to check!

            Show
            dobedobedoh Andrew Nicols added a comment - Bah - thanks Mary. I must have opened the m25 link twice when I opened into new tabs to check!
            Hide
            lazydaisy Mary Evans added a comment -

            A little like spaghetti junction! M25 and all that jazz!

            Thanks for fixing.

            Show
            lazydaisy Mary Evans added a comment - A little like spaghetti junction! M25 and all that jazz! Thanks for fixing.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Integrated (25 and master), thanks!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Integrated (25 and master), thanks!
            Hide
            abgreeve Adrian Greeve added a comment -

            Sorry, but the buttons don't end up next to each other, they are always below each other.

            • I checked that the caches had been purged.
            • I had a look at the diff and made sure that I had the latest integration downloaded.
            • I tried on Fred's machine.

            Still no joy with the buttons.

            Show
            abgreeve Adrian Greeve added a comment - Sorry, but the buttons don't end up next to each other, they are always below each other. I checked that the caches had been purged. I had a look at the diff and made sure that I had the latest integration downloaded. I tried on Fred's machine. Still no joy with the buttons.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Grrr, it's #page, not #path (line #114 of theme/base/style/admin.css)

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Grrr, it's #page, not #path (line #114 of theme/base/style/admin.css)
            Hide
            lazydaisy Mary Evans added a comment -

            Is it too late to fix?

            Show
            lazydaisy Mary Evans added a comment - Is it too late to fix?
            Show
            lazydaisy Mary Evans added a comment - @ Eloy Lafuente (stronk7) Corrected NEW branches here if it helps? https://github.com/lazydaisy/moodle/compare/master...MDL-40376_master https://github.com/lazydaisy/moodle/compare/master...MDL-40376_M25
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment - - edited

            Hi Mary, yes that is correct, but it conflicts with currently landed code. If it's ok, I'll add a new commit on top of current integration.git just changing the wrong #path with the correct #page.

            This is the (unique) line to change, I think (@ current integration.git):

            http://git.moodle.org/gw?p=integration.git;a=blob;f=theme/base/style/admin.css;h=7ca7bb2ad04fc4f6f59d50538139973be87947d0;hb=refs/heads/master#l114

            BTW... what happens with BS themes? Do they expose the same problem?

            Ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - - edited Hi Mary, yes that is correct, but it conflicts with currently landed code. If it's ok, I'll add a new commit on top of current integration.git just changing the wrong #path with the correct #page. This is the (unique) line to change, I think (@ current integration.git): http://git.moodle.org/gw?p=integration.git;a=blob;f=theme/base/style/admin.css;h=7ca7bb2ad04fc4f6f59d50538139973be87947d0;hb=refs/heads/master#l114 BTW... what happens with BS themes? Do they expose the same problem? Ciao
            Hide
            lazydaisy Mary Evans added a comment -

            That's OK...just as long as it is fixed.

            I need to check Clean theme. It may need fixing...not sure...but I can fix it if necessary.

            Show
            lazydaisy Mary Evans added a comment - That's OK...just as long as it is fixed. I need to check Clean theme. It may need fixing...not sure...but I can fix it if necessary.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Added 1 commit on top fixing the #path => #page typo. Now works after purging caches.

            Incidentally I had the clean theme installed in my 25_STABLE branch and I can confirm the same problem happens there (buttons are shown vertically separated).

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Added 1 commit on top fixing the #path => #page typo. Now works after purging caches. Incidentally I had the clean theme installed in my 25_STABLE branch and I can confirm the same problem happens there (buttons are shown vertically separated).
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            So, if you can provide a patch for Bootstrap... that would be great. Or do you prefer to move that to different issue?

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - So, if you can provide a patch for Bootstrap... that would be great. Or do you prefer to move that to different issue?
            Hide
            lazydaisy Mary Evans added a comment -

            Sorry I got called away. Back now to sort this out, but as it is not as pressing as Moodle standard themes I'll fix this in separate issue.

            Thanks

            Show
            lazydaisy Mary Evans added a comment - Sorry I got called away. Back now to sort this out, but as it is not as pressing as Moodle standard themes I'll fix this in separate issue. Thanks
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Great so MDL-40485 will handle Bootstrap themes... sending this back to testing.

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Great so MDL-40485 will handle Bootstrap themes... sending this back to testing.
            Hide
            abgreeve Adrian Greeve added a comment -

            Success!
            Tested on 2.5 and master.
            Buttons are in their correct positions.

            Show
            abgreeve Adrian Greeve added a comment - Success! Tested on 2.5 and master. Buttons are in their correct positions.
            Hide
            damyon Damyon Wiese added a comment -

            This issue is fixed! Hurray! Hurray!
            Your issue is fixed, what a wonderful day!

            Cheers, Damyon

            Show
            damyon Damyon Wiese added a comment - This issue is fixed! Hurray! Hurray! Your issue is fixed, what a wonderful day! Cheers, Damyon

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  8/Jul/13