Moodle
  1. Moodle
  2. MDL-28692

Some themes do not highlight forced-settings in the admin UI

    Details

    • Database:
      Any
    • Testing Instructions:
      Hide
      1. ADD the following to your moodle root config.php
        $CFG->themedesignermode = '1';
      2. NEXT go to Site administration > Appearance > Themes > Theme settings
      3. You should find written, just below Theme designer mode, "Defined in config.php" on a yellow background.
      4. Retest in all core themes
      Show
      ADD the following to your moodle root config.php $CFG->themedesignermode = '1'; NEXT go to Site administration > Appearance > Themes > Theme settings You should find written, just below Theme designer mode, "Defined in config.php" on a yellow background. Retest in all core themes
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull 2.4 Branch:
      MDL-28692_M24
    • Pull Master Branch:
      MDL-28692_master
    • Rank:
      18353

      Description

      This was corrected in formal white as part of MDL-28575. While testing that "Brick" does not highlight hard coded theme settings.

      We just need to check all of our standard themes and make sure that they all do it.

        Issue Links

          Activity

          Hide
          Mary Evans added a comment -

          Andrew,
          Do you know which themes are affected? Which theme or theme did you test for this?

          Show
          Mary Evans added a comment - Andrew, Do you know which themes are affected? Which theme or theme did you test for this?
          Hide
          Andrew Davis added a comment -

          The only theme I tested was "brick" and it didn't highlight.

          Show
          Andrew Davis added a comment - The only theme I tested was "brick" and it didn't highlight.
          Hide
          Mary Evans added a comment -

          Thanks Andrew, I'll check Base theme out to see if there is provision for this in the CSS, which I doubt, if not I'll add it as this will be sufficient to get it into all themes.

          There's so many different things happening to Moodle which are affecting the way themes work, which we don't get told about. How are we supposed to keep up. I didn't even know one could hard code to have Theme Designer Mode on! The first I heard about it was the other day when Daniele added a patch for it in Formal White. And then, in my naivety, assumed it was just for the Formal white theme.

          Cheers
          Mary

          Show
          Mary Evans added a comment - Thanks Andrew, I'll check Base theme out to see if there is provision for this in the CSS, which I doubt, if not I'll add it as this will be sufficient to get it into all themes. There's so many different things happening to Moodle which are affecting the way themes work, which we don't get told about. How are we supposed to keep up. I didn't even know one could hard code to have Theme Designer Mode on! The first I heard about it was the other day when Daniele added a patch for it in Formal White. And then, in my naivety, assumed it was just for the Formal white theme. Cheers Mary
          Hide
          Andrew Davis added a comment -

          Moodle is evolving so quickly its tough to keep on top of even for someone in HQ :| Its great to see so many new features being created but it is pretty hectic.

          Show
          Andrew Davis added a comment - Moodle is evolving so quickly its tough to keep on top of even for someone in HQ :| Its great to see so many new features being created but it is pretty hectic.
          Hide
          Mary Evans added a comment -

          I never really understood how this feature slipped through without it being styled, that's assuming it should be styled. If seems to me that this should have been done at the time the feature was introduced, and the style added to Base theme.

          Show
          Mary Evans added a comment - I never really understood how this feature slipped through without it being styled, that's assuming it should be styled. If seems to me that this should have been done at the time the feature was introduced, and the style added to Base theme.
          Hide
          Mary Evans added a comment -

          All that testing for this tiny feature!

          Show
          Mary Evans added a comment - All that testing for this tiny feature!
          Hide
          Dan Poltawski added a comment -

          Hi Mary,

          Sorry - this change isn't correct right.

          The feature has been there for a long time (at least since 1.9) and its possible to override any config setting in config.php, not only theme designer mode. Its docmented in '9. FORCED SETTINGS' in config-dist.php.

          Your change seems only to be changing the behaviour of themedesignermode being override. But for example, in my config.php I have

          $CFG->debugdisplay = 1;
          

          So, this change needs to be applied to any 'form-overriden' setting in the admin tree I think. I can see some CSS defined for it in standard and not in base. Could this have been done because of the use of colour? (Not able to make good colour decision for all themeS?) I'm not sure, just speculating.

          Show
          Dan Poltawski added a comment - Hi Mary, Sorry - this change isn't correct right. The feature has been there for a long time (at least since 1.9) and its possible to override any config setting in config.php, not only theme designer mode. Its docmented in '9. FORCED SETTINGS' in config-dist.php. Your change seems only to be changing the behaviour of themedesignermode being override. But for example, in my config.php I have $CFG->debugdisplay = 1; So, this change needs to be applied to any 'form-overriden' setting in the admin tree I think. I can see some CSS defined for it in standard and not in base. Could this have been done because of the use of colour? (Not able to make good colour decision for all themeS?) I'm not sure, just speculating.
          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 -

          That's because this ONLY relates to Theme Designer Mode as this is important that people see that it hard-coded. I couldn't care less for any other setting. This is why the Test Instructions tell you to add $CFG->themedesignermode = '1'; and that is why I styled it the way I did.

          Thanks
          Mary

          Show
          Mary Evans added a comment - That's because this ONLY relates to Theme Designer Mode as this is important that people see that it hard-coded. I couldn't care less for any other setting. This is why the Test Instructions tell you to add $CFG->themedesignermode = '1'; and that is why I styled it the way I did. Thanks Mary
          Hide
          Mary Evans added a comment -

          Closing this as it is not widely recognised in Moodle to highlight hard coded settings. If it was then this would have been done in Moodle 1.9 and carried forward into Moodle 2+

          Show
          Mary Evans added a comment - Closing this as it is not widely recognised in Moodle to highlight hard coded settings. If it was then this would have been done in Moodle 1.9 and carried forward into Moodle 2+
          Hide
          Dan Poltawski added a comment -

          Hi Mary,

          I don't agree. I think this is a bug for all the settings, we have lots of long standing bugs. I'm sorry that you didn't agree with my reopening of it, but I think we should solve this properly.

          I've assigned it to moodle.com, but feel free to take it if you are interested.

          Show
          Dan Poltawski added a comment - Hi Mary, I don't agree. I think this is a bug for all the settings, we have lots of long standing bugs. I'm sorry that you didn't agree with my reopening of it, but I think we should solve this properly. I've assigned it to moodle.com, but feel free to take it if you are interested.
          Hide
          Mary Evans added a comment - - edited

          Dan,
          I was playing with these settings and had NO idea what they were or what they did, or could do for that matter. I honestly thought it only related to the Theme Designer Mode. Now that the bulb lit up and the penny dropped, I'm on cloud nine! Did you know, you can actually add menu items in root/config.php using $CFG->custommenuitems = 'Home|http://moodlesiteurl|Home|en';
          This is absolutely fantastic!!! So I have taken MDL-28692 back and will fix it as you suggested.

          Cheers

          PS: Sorry for the misunderstanding.

          Show
          Mary Evans added a comment - - edited Dan, I was playing with these settings and had NO idea what they were or what they did, or could do for that matter. I honestly thought it only related to the Theme Designer Mode. Now that the bulb lit up and the penny dropped, I'm on cloud nine! Did you know, you can actually add menu items in root/config.php using $CFG->custommenuitems = 'Home|http://moodlesiteurl|Home|en'; This is absolutely fantastic!!! So I have taken MDL-28692 back and will fix it as you suggested. Cheers PS: Sorry for the misunderstanding.
          Hide
          Dan Poltawski added a comment -

          Thanks for taking it Mary

          Show
          Dan Poltawski added a comment - Thanks for taking it Mary
          Hide
          Eloy Lafuente (stronk7) added a comment - - edited

          Hi Mary,

          1) I've amended the title a bit.

          2) If we are adding this to base... shouldn't we take rid of it in formal white and, also in standard? More yet, "yellow" or "#ff6" ?

          3) It would be great to amend instructions a bit to cover some different (canvas based, standard..) themes in the tests.

          4) It's not clear for me this being a bug. Perhaps it would be enough to apply it only to master? (not 100% sure, just a thought, you decide - Mary/Integrator).

          Show
          Eloy Lafuente (stronk7) added a comment - - edited Hi Mary, 1) I've amended the title a bit. 2) If we are adding this to base... shouldn't we take rid of it in formal white and, also in standard? More yet, "yellow" or "#ff6" ? 3) It would be great to amend instructions a bit to cover some different (canvas based, standard..) themes in the tests. 4) It's not clear for me this being a bug. Perhaps it would be enough to apply it only to master? (not 100% sure, just a thought, you decide - Mary/Integrator).
          Hide
          Mary Evans added a comment -

          It's not going to do any harm if it went into Base for STABLE_BACKLOG is it? I can open a new tracker remove it from Formal White and Standard no problem.

          Show
          Mary Evans added a comment - It's not going to do any harm if it went into Base for STABLE_BACKLOG is it? I can open a new tracker remove it from Formal White and Standard no problem.
          Hide
          Damyon Wiese added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          Thanks!

          Show
          Damyon Wiese added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. Thanks!
          Hide
          Mary Evans added a comment -

          All branches rebased.

          Show
          Mary Evans added a comment - All branches rebased.
          Hide
          Mary Evans added a comment -

          @Eloy (El supremo) to answer your questions it's easier if you read the discussion between Dan and myself. As you will see I did not think it that important, but worthy enough to be added to Base theme. But I only styled it to be used for the Theme Designer Mode as the Test instructions suggest. Dan made me see a differnt side of the coin as it can affect lots of other Admin settings too.

          Anyway, Dan thinks it is a Bug, so bug it is! And as such need fixing in stable_backlog branches.

          Show
          Mary Evans added a comment - @Eloy (El supremo) to answer your questions it's easier if you read the discussion between Dan and myself. As you will see I did not think it that important, but worthy enough to be added to Base theme. But I only styled it to be used for the Theme Designer Mode as the Test instructions suggest. Dan made me see a differnt side of the coin as it can affect lots of other Admin settings too. Anyway, Dan thinks it is a Bug, so bug it is! And as such need fixing in stable_backlog branches.
          Hide
          Damyon Wiese added a comment -

          Thanks Mary,

          I have added this to 23, 24 and master.

          I created a new issue for removing the css from standard and formal_white MDL-38183.

          Show
          Damyon Wiese added a comment - Thanks Mary, I have added this to 23, 24 and master. I created a new issue for removing the css from standard and formal_white MDL-38183 .
          Hide
          Andrew Davis added a comment -

          I think its all working as it should now. Passing.

          Show
          Andrew Davis added a comment - I think its all working as it should now. Passing.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Because

          A
          MARVELOUS
          A       U
          Z  YOU  P
          I  ARE  E
          N  PPL  R
          G       B
            TNKS! 
          

          Closing, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Because A MARVELOUS A U Z YOU P I ARE E N PPL R G B TNKS! Closing, ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: