Moodle
  1. Moodle
  2. MDL-38856 META: Issues around Bootstrapbase and Clean theme
  3. MDL-39697

Bootstrapbase $THEME->layouts need 'Maintenance' and is missing important options.

    Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 2.5
    • Fix Version/s: 2.5.1
    • Component/s: Themes
    • Labels:
    • Rank:
      50428

      Description

      After a chance comment by Jason Fowler in MDL-39573, and on inspection of bootstrapbase/config.php, later revealed that the maintenance layout was incorrect and missing important options namely:

       'maintenance' => array(
              'file' => 'general.php',
              'regions' => array(),
              'options' => array('noblocks' => true, 'nofooter' => true, 'nonavbar' => true, 'nocustommenu' => true, 'nocourseheaderfooter' => true),
          ),
      

      Also comparing it with base theme showed up other settings to.

      That said, the intention of this issue is to add Base $THEME->layouts so that some continuity is maintained for future versions of Moodle.

        Issue Links

          Activity

          Hide
          Damyon Wiese added a comment -

          Hi Mary,

          Please add testing instructions and resubmit this for integration.

          Thanks!

          Show
          Damyon Wiese added a comment - Hi Mary, Please add testing instructions and resubmit this for integration. Thanks!
          Hide
          CiBoT added a comment -

          Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

          Show
          CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
          Hide
          Mary Evans added a comment -

          Sorry I totally forgot about this as I was so very busy this weekend.

          Show
          Mary Evans added a comment - Sorry I totally forgot about this as I was so very busy this weekend.
          Hide
          Mary Evans added a comment -

          This now blocks MDL-39758 which I have just fixed and pushed for integration.

          Show
          Mary Evans added a comment - This now blocks MDL-39758 which I have just fixed and pushed for integration.
          Hide
          Damyon Wiese added a comment -

          Hi Mary,

          Thanks for the testing instructions - they look fine.

          I compared the config with the config for base and the frontpage has an extra nonavbar option in bootstrapbase. I checked with Barbara and this is not desired as we want to have that element showing by default for consistency (esp on mobile where we are thinking about how to use the navbar more effectively).

          If there is a definite reason for leaving it out please let me know.

          (I'm reopening this issue while we discuss this).

          I would also like to know your thoughts on whether the clean theme now needs to override the embedded layout from bootstrapbase to implement the custom footer/header and whether we also need to add custom layouts for report/maintenance pages. I added a new issue related to this as I think we should remove the custom layouts from clean altogether (MDL-39777).

          • Damyon
          Show
          Damyon Wiese added a comment - Hi Mary, Thanks for the testing instructions - they look fine. I compared the config with the config for base and the frontpage has an extra nonavbar option in bootstrapbase. I checked with Barbara and this is not desired as we want to have that element showing by default for consistency (esp on mobile where we are thinking about how to use the navbar more effectively). If there is a definite reason for leaving it out please let me know. (I'm reopening this issue while we discuss this). I would also like to know your thoughts on whether the clean theme now needs to override the embedded layout from bootstrapbase to implement the custom footer/header and whether we also need to add custom layouts for report/maintenance pages. I added a new issue related to this as I think we should remove the custom layouts from clean altogether ( MDL-39777 ). Damyon
          Hide
          CiBoT added a comment -

          Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

          Show
          CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
          Hide
          Mary Evans added a comment -

          Damyon,

          Can you not make the changes that are required while integrating this issue?

          And NO Clean does not need to have a seperate embedded.php as this file is empty, that's the nature of the embedded.php. It has no header no footer, no need for customisation. It's basically a box to carry an object, in other words an empty shell. I could get poetic here. LOL

          Cheers
          Mary

          Show
          Mary Evans added a comment - Damyon, Can you not make the changes that are required while integrating this issue? And NO Clean does not need to have a seperate embedded.php as this file is empty, that's the nature of the embedded.php. It has no header no footer, no need for customisation. It's basically a box to carry an object, in other words an empty shell. I could get poetic here. LOL Cheers Mary
          Hide
          Mary Evans added a comment -

          @Damyon,

          I forgot that the frontpage layout in Bootstrapbase uses general.php and as such needs the nonavbar=true option. So can you add this back as desired? If not I will try and get this done tonight.

          Cheers
          Mary

          Show
          Mary Evans added a comment - @Damyon, I forgot that the frontpage layout in Bootstrapbase uses general.php and as such needs the nonavbar=true option. So can you add this back as desired? If not I will try and get this done tonight. Cheers Mary
          Hide
          Damyon Wiese added a comment -

          Yes - I can add the changes in integration - but I just wanted you to confirm that you agree. I'll pull this back in in a few minutes and add the changes.

          Show
          Damyon Wiese added a comment - Yes - I can add the changes in integration - but I just wanted you to confirm that you agree. I'll pull this back in in a few minutes and add the changes.
          Hide
          Damyon Wiese added a comment -

          Thanks Mary,

          I've integrated this as is (the patch has the nonavbar option for the frontpage already).

          Integrated to 25 and master.

          Note: I questioned adding this to 25 (stable) with the other integrators - but 2.5 has only just been released and I think until 2.5.1 it is worth adding some fixes like this before lots of people start writing bootstrap themes.

          Show
          Damyon Wiese added a comment - Thanks Mary, I've integrated this as is (the patch has the nonavbar option for the frontpage already). Integrated to 25 and master. Note: I questioned adding this to 25 (stable) with the other integrators - but 2.5 has only just been released and I think until 2.5.1 it is worth adding some fixes like this before lots of people start writing bootstrap themes.
          Hide
          Mary Evans added a comment -

          Thanks...
          I was just about to fix this and now see you have done it. Great

          Show
          Mary Evans added a comment - Thanks... I was just about to fix this and now see you have done it. Great
          Hide
          Adrian Greeve added a comment -

          Layout looks like it's part of the Clean theme.
          No blocks were present.
          No links were displayed in the footer.
          Test passed.

          Show
          Adrian Greeve added a comment - Layout looks like it's part of the Clean theme. No blocks were present. No links were displayed in the footer. Test passed.
          Hide
          Damyon Wiese added a comment -

          Thanks for your contribution! This issue has been reviewed, integrated, tested and now released to everyone.

          Closing as Fixed!

          Show
          Damyon Wiese added a comment - Thanks for your contribution! This issue has been reviewed, integrated, tested and now released to everyone. Closing as Fixed!

            People

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

              Dates

              • Created:
                Updated:
                Resolved: