Uploaded image for project: '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 Master Branch:
      MDL-28692_master

      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.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            lazydaisy Mary Evans added a comment -

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

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

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

            Show
            andyjdavis Andrew Davis added a comment - The only theme I tested was "brick" and it didn't highlight.
            Hide
            lazydaisy 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
            lazydaisy 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
            andyjdavis 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
            andyjdavis 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
            lazydaisy 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
            lazydaisy 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
            lazydaisy Mary Evans added a comment -

            All that testing for this tiny feature!

            Show
            lazydaisy Mary Evans added a comment - All that testing for this tiny feature!
            Hide
            poltawski 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
            poltawski 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 CiBoT added a comment -

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

            Show
            cibot CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
            Hide
            lazydaisy 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
            lazydaisy 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
            lazydaisy 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
            lazydaisy 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
            poltawski 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
            poltawski 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
            lazydaisy 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
            lazydaisy 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
            poltawski Dan Poltawski added a comment -

            Thanks for taking it Mary

            Show
            poltawski Dan Poltawski added a comment - Thanks for taking it Mary
            Hide
            stronk7 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
            stronk7 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
            lazydaisy 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
            lazydaisy 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 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 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
            lazydaisy Mary Evans added a comment -

            All branches rebased.

            Show
            lazydaisy Mary Evans added a comment - All branches rebased.
            Hide
            lazydaisy 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
            lazydaisy 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 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 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
            andyjdavis Andrew Davis added a comment -

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

            Show
            andyjdavis Andrew Davis added a comment - I think its all working as it should now. Passing.
            Hide
            stronk7 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
            stronk7 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:
                  Fix Release Date:
                  11/Mar/13