Moodle
  1. Moodle
  2. MDL-40376

Padding at top of /course/manage.php

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor 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 2.5 Branch:
      MDL-40376-m25
    • Pull Master Branch:
    • Rank:
      51159

      Description

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

        Issue Links

          Activity

          Hide
          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
          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
          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
          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
          Mary Evans added a comment -

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

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

          Please amend and then submit for integration.

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

          Updated as per Mary Evans suggestions.

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

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

          Show
          Mary Evans added a comment - Andrew, did you actually push the changes to origin as they are not visible in your current commits.
          Hide
          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
          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
          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
          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
          Mary Evans added a comment -

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

          Show
          Mary Evans added a comment - M25 branch is OK only Master branch is showing old commit!
          Hide
          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
          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
          Mary Evans added a comment -

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

          Thanks for fixing.

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

          Integrated (25 and master), thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Integrated (25 and master), thanks!
          Hide
          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
          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
          Eloy Lafuente (stronk7) added a comment -

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

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

          Is it too late to fix?

          Show
          Mary Evans added a comment - Is it too late to fix?
          Show
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          Eloy Lafuente (stronk7) added a comment -

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

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

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

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

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

          Cheers, Damyon

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