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

          Attachments

            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