Moodle
  1. Moodle
  2. MDL-29723

Black text appears on dark blue background and is hard to read

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.1.2
    • Fix Version/s: 2.3.5, 2.4.2
    • Component/s: Themes
    • Labels:
    • Environment:
      Ubuntu 10.04
    • Testing Instructions:
      Hide
      1. Select Standard theme from Theme selector.
      2. In a course create a quiz.
      3. Create a question and TEST that the Question Box Window has white text against a dark blue background.
      4. With the page still open TEST all themes. Start off by adding &theme=afterburner at the end of the page URL, changing the theme name for each theme in Moodle core.
        Full list: afterburner, anomaly, arialist, base, binarius, boxxie, brick, canvas, formfactor, fusion, leatherbound, magazine, nimble, nonzero, overlay, serenity, sky_high, splash, standardold
        Please Note: to see both Base & Canvas themes you need Theme Designer Mode enabled. (under settings: Theme settings)
      Show
      Select Standard theme from Theme selector. In a course create a quiz. Create a question and TEST that the Question Box Window has white text against a dark blue background. With the page still open TEST all themes. Start off by adding &theme=afterburner at the end of the page URL, changing the theme name for each theme in Moodle core. Full list: afterburner, anomaly, arialist, base, binarius, boxxie, brick, canvas, formfactor, fusion, leatherbound, magazine, nimble, nonzero, overlay, serenity, sky_high, splash, standardold Please Note: to see both Base & Canvas themes you need Theme Designer Mode enabled. (under settings: Theme settings)
    • Affected Branches:
      MOODLE_21_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull 2.4 Branch:
      wip-MDL-29723_M24
    • Pull Master Branch:
      wip-MDL-29723_master
    • Rank:
      19237

      Description

      After creating a question on a quiz, a text box containing "Question bank contents" appears on the top right. It is impossible to read unless you select the text. It may be the quiz component or it could be the whole theme I was using at the time (Leatherbound).

        Issue Links

          Activity

          Hide
          Tim Hunt added a comment -

          Leatherbound theme issue. Re-assigning.

          Show
          Tim Hunt added a comment - Leatherbound theme issue. Re-assigning.
          Hide
          Mary Evans added a comment -

          Hi Tim,

          If I find this is a QUIZ style issue can I change the stylesheet in QUIZ?

          Show
          Mary Evans added a comment - Hi Tim, If I find this is a QUIZ style issue can I change the stylesheet in QUIZ?
          Hide
          Mary Evans added a comment -

          Just revisiting this and wondering why it is that your CSS in quiz/style.css is not filtering though to the themes. It matters not what the questionbankwindow looks like as long as it works as is, regardless of theme colours. I like it blue actually as it look distinctive and people know what it is.

          I think the problem stems from the way you wrote the CSS.

          For example: in mod/quiz/edit.php I found these lines...

          echo '<div class="questionbankwindow ' . $bankclass . 'block">';
          echo '<div class="header"><div class="title"><h2>';
          

          which shows the header title is in actual fact a block header title h2 which in most themes uses the default color which is black.

          So what we need to do is write this correctly in quiz/style.css as...

          .questionbankwindow.block .header .title h2 { color: white;}
          

          That way it will work, and all in the Moodle garden would be rosey!

          Do you want to do this Tim, or shall I?

          Show
          Mary Evans added a comment - Just revisiting this and wondering why it is that your CSS in quiz/style.css is not filtering though to the themes. It matters not what the questionbankwindow looks like as long as it works as is, regardless of theme colours. I like it blue actually as it look distinctive and people know what it is. I think the problem stems from the way you wrote the CSS. For example: in mod/quiz/edit.php I found these lines... echo '<div class= "questionbankwindow ' . $bankclass . 'block" >'; echo '<div class= "header" ><div class= "title" ><h2>'; which shows the header title is in actual fact a block header title h2 which in most themes uses the default color which is black. So what we need to do is write this correctly in quiz/style.css as... .questionbankwindow.block .header .title h2 { color: white;} That way it will work, and all in the Moodle garden would be rosey! Do you want to do this Tim, or shall I?
          Hide
          Mary Evans added a comment -

          @Tim
          If you have time, could you Peer Review my work here?
          Many thanks
          Mary

          Show
          Mary Evans added a comment - @Tim If you have time, could you Peer Review my work here? Many thanks Mary
          Hide
          Mary Evans added a comment -

          Attached image showing changes.

          Show
          Mary Evans added a comment - Attached image showing changes.
          Hide
          Tim Hunt added a comment -

          The image does not seem to match the code. The image shows the square brackets still present, but you removed them from the code, and I don't understand why.

          There are also other aspects of the change where I cannot deduce why you did it that way, and you have not explained either in the commit comment, or here. I am sure you have good reasons, I just can't work them out.

          Show
          Tim Hunt added a comment - The image does not seem to match the code. The image shows the square brackets still present, but you removed them from the code, and I don't understand why. There are also other aspects of the change where I cannot deduce why you did it that way, and you have not explained either in the commit comment, or here. I am sure you have good reasons, I just can't work them out.
          Hide
          Mary Evans added a comment - - edited

          The square brackets are still in the code, but on the outside of the link, so when you click the link IT changes color NOT the brackets. I added some extra css to style that particular link so it changes color and text-decoration on hover. Plus added a margin to separate the input checkboxes from the text, which is one of the things we are asked to do a lot in Themes. All the question checkboxes and list numbers/letters in Quiz could do with some right margins to do the same. Plus doing the fixes at source saves on extra CSS in the themes, we are already overloaded.

          Show
          Mary Evans added a comment - - edited The square brackets are still in the code, but on the outside of the link, so when you click the link IT changes color NOT the brackets. I added some extra css to style that particular link so it changes color and text-decoration on hover. Plus added a margin to separate the input checkboxes from the text, which is one of the things we are asked to do a lot in Themes. All the question checkboxes and list numbers/letters in Quiz could do with some right margins to do the same. Plus doing the fixes at source saves on extra CSS in the themes, we are already overloaded.
          Hide
          Mary Evans added a comment -

          Further to my last comment you really need to test it in a browser.

          Show
          Mary Evans added a comment - Further to my last comment you really need to test it in a browser.
          Hide
          Tim Hunt added a comment -

          That makes sense. Thanks. Clearly I need to look more closely, which is will do tomorrow.

          Show
          Tim Hunt added a comment - That makes sense. Thanks. Clearly I need to look more closely, which is will do tomorrow.
          Hide
          Tim Hunt added a comment -

          OK. Moving the square brackets outside the link: good.

          Making the title centred? I was not sure at first, but I think you are right.

          I don't yet understand what .questionbankwindow.block input

          {margin:0 5px;}

          is doing for us.

          There is a problem in formalwhite (which I was only looking at by chance). Your changes mean that the header text is now white on a pale grey gradient, but I can't work out why.

          I guess that means we really need to test this in all core themes, which is a bore.

          Why is nothing ever easy?

          Show
          Tim Hunt added a comment - OK. Moving the square brackets outside the link: good. Making the title centred? I was not sure at first, but I think you are right. I don't yet understand what .questionbankwindow.block input {margin:0 5px;} is doing for us. There is a problem in formalwhite (which I was only looking at by chance). Your changes mean that the header text is now white on a pale grey gradient, but I can't work out why. I guess that means we really need to test this in all core themes, which is a bore. Why is nothing ever easy?
          Hide
          Mary Evans added a comment -

          Test it is Standard them...Formal White is not the best for this as there could well be differences in the CSS.

          The input is adding 5px to both left and right sides of the checkboxes in the QBwindow. Nothing more and certainly NOT white on grey! as you will see in Standard theme.

          Show
          Mary Evans added a comment - Test it is Standard them...Formal White is not the best for this as there could well be differences in the CSS. The input is adding 5px to both left and right sides of the checkboxes in the QBwindow. Nothing more and certainly NOT white on grey! as you will see in Standard theme.
          Hide
          Tim Hunt added a comment -

          Well, before we can submit this for integration, we need to make sure we have not made things worse in any core theme.

          The extra margin around those checkboxes probably is a bit more attractive, but we have a very limited amount of space, and the most useful information is the question names. I do not think we should reduce the space available there.

          Also, there are lots of other inputs around, and the extra margin will apply to them too. Is that intended?

          Show
          Tim Hunt added a comment - Well, before we can submit this for integration, we need to make sure we have not made things worse in any core theme. The extra margin around those checkboxes probably is a bit more attractive, but we have a very limited amount of space, and the most useful information is the question names. I do not think we should reduce the space available there. Also, there are lots of other inputs around, and the extra margin will apply to them too. Is that intended?
          Hide
          Mary Evans added a comment -

          That's OK I can/will test CORE themes to see what the outcome is and perahsp deal with it in the theme, but from memory I don't think the Question Bank has been styled in any CORE themes other than Formal White which I donlt have much to do with, so Daniele may have styled it. We shall see.

          Other than the input, which can be styled just to target the ones I was intending the fix for, are you happy with the look of it so far?

          Show
          Mary Evans added a comment - That's OK I can/will test CORE themes to see what the outcome is and perahsp deal with it in the theme, but from memory I don't think the Question Bank has been styled in any CORE themes other than Formal White which I donlt have much to do with, so Daniele may have styled it. We shall see. Other than the input, which can be styled just to target the ones I was intending the fix for, are you happy with the look of it so far?
          Hide
          Tim Hunt added a comment -

          I'm wondering if we need to find a time to discuss this synchronously in developers' chat? What time would be convenient for you?

          Show
          Tim Hunt added a comment - I'm wondering if we need to find a time to discuss this synchronously in developers' chat? What time would be convenient for you?
          Hide
          Mary Evans added a comment -

          Now?
          Any time really, within reason. I'm free most of tomorrow, so I could make some time, whatever is convenient for you?

          Show
          Mary Evans added a comment - Now? Any time really, within reason. I'm free most of tomorrow, so I could make some time, whatever is convenient for you?
          Hide
          Tim Hunt added a comment -

          Now works for me! I have chat open.

          Show
          Tim Hunt added a comment - Now works for me! I have chat open.
          Hide
          Mary Evans added a comment -

          Tim,

          What did we decide abut this, can you remember?

          Show
          Mary Evans added a comment - Tim, What did we decide abut this, can you remember?
          Hide
          Tim Hunt added a comment -

          I think we agreed:

          1. HTML changes good.

          2. revert the .questionbankwindow.block input

          {margin:0 5px;}

          change. Space is at too much of a premium on this screen to worry about things looking cramped.

          3. Other CSS changes were OK.

          However, I have just spotted:

          4. Surely this combination:

          ... div.header {background-color:#009; ...

          ... a#hidebankcmd:hover {color:#009; ...

          cannot be right. It will make the link invisible when you are hovering. I suggest changing the :hover rule something like #00f (but you are probably better at choosing colours than me.)

          I think the other thing to do was to see how this looks in all themes.

          Show
          Tim Hunt added a comment - I think we agreed: 1. HTML changes good. 2. revert the .questionbankwindow.block input {margin:0 5px;} change. Space is at too much of a premium on this screen to worry about things looking cramped. 3. Other CSS changes were OK. However, I have just spotted: 4. Surely this combination: ... div.header {background-color:#009; ... ... a#hidebankcmd:hover {color:#009; ... cannot be right. It will make the link invisible when you are hovering. I suggest changing the :hover rule something like #00f (but you are probably better at choosing colours than me.) I think the other thing to do was to see how this looks in all themes.
          Hide
          Mary Evans added a comment - - edited

          Agree with 1,2 & 3.

          Re: No.4
          Notice that the hover adds a white background too. so is in effect blue text on white b/ground.

          #page-mod-quiz-edit .questionbankwindow a#showbankcmd:hover,
          #page-mod-quiz-edit .questionbankwindow a#hidebankcmd:hover {
              color:#009;background-color:#fff;text-decoration:none;
          }
          Show
          Mary Evans added a comment - - edited Agree with 1,2 & 3. Re: No.4 Notice that the hover adds a white background too. so is in effect blue text on white b/ground. #page-mod-quiz-edit .questionbankwindow a#showbankcmd:hover, #page-mod-quiz-edit .questionbankwindow a#hidebankcmd:hover { color:#009;background-color:#fff;text-decoration:none; }
          Hide
          Tim Hunt added a comment -

          Sorry, I was not paying enough attention.

          Show
          Tim Hunt added a comment - Sorry, I was not paying enough attention.
          Hide
          Mary Evans added a comment -

          Moodle does that to you after a while. I just let my hot mug of milky-coffee go stone cold because I was so wrapped up in Moodling!

          Show
          Mary Evans added a comment - Moodle does that to you after a while. I just let my hot mug of milky-coffee go stone cold because I was so wrapped up in Moodling!
          Hide
          Mary Evans added a comment -

          In trying to update this I seem to have created more commits! I hope it works OK when it comes to integration?

          Show
          Mary Evans added a comment - In trying to update this I seem to have created more commits! I hope it works OK when it comes to integration?
          Hide
          Tim Hunt added a comment -

          Wow! you have been really creative there Mary

          If you want to clean that up, then there are various options (there are always various options with git.) One would be 'git rebase -i master' to fix it with an interactive rebase.

          The other option, probably simpler, is

          git checkout -b MDL-29723_master_new
          git merge --squash MDL-29723_master
          git commit
          
          Show
          Tim Hunt added a comment - Wow! you have been really creative there Mary If you want to clean that up, then there are various options (there are always various options with git.) One would be 'git rebase -i master' to fix it with an interactive rebase. The other option, probably simpler, is git checkout -b MDL-29723_master_new git merge --squash MDL-29723_master git commit
          Hide
          Mary Evans added a comment -

          I might try this later as I am off to B&Q to spend Gift Vouchers before they too, like HMV, decide to go Bankrupt!

          Show
          Mary Evans added a comment - I might try this later as I am off to B&Q to spend Gift Vouchers before they too, like HMV, decide to go Bankrupt!
          Hide
          Mary Evans added a comment -

          Thanks Tim that worked a treat! It looks tons better now.

          Show
          Mary Evans added a comment - Thanks Tim that worked a treat! It looks tons better now.
          Hide
          Tim Hunt added a comment -

          Yay! Thanks. +1 from me as peer-reviewer.

          Show
          Tim Hunt added a comment - Yay! Thanks. +1 from me as peer-reviewer.
          Hide
          Mary Evans added a comment -

          Tim,
          Do you think this would be OK to back-port to Moodle 2.2, 2.3 & 2.4 or just leave it at 2.5(master)?

          Show
          Mary Evans added a comment - Tim, Do you think this would be OK to back-port to Moodle 2.2, 2.3 & 2.4 or just leave it at 2.5(master)?
          Hide
          Tim Hunt added a comment -

          This is a bug, so should be back-ported to 2.3+. (2.2 is no longer supported apart from security issues.)

          Show
          Tim Hunt added a comment - This is a bug, so should be back-ported to 2.3+. (2.2 is no longer supported apart from security issues.)
          Hide
          Mary Evans added a comment - - edited

          All done!

          Now I just have to wait until Moodle is updated then I'll need to re-base before next pull.

          Tim, is there a quick way to rebase a number of branches, as in a batch? Or has each one to be done separately?

          Show
          Mary Evans added a comment - - edited All done! Now I just have to wait until Moodle is updated then I'll need to re-base before next pull. Tim, is there a quick way to rebase a number of branches, as in a batch? Or has each one to be done separately?
          Hide
          Tim Hunt added a comment -

          I wrote about the way I do this here: http://tjhunt.blogspot.co.uk/2012/08/automating-git.html

          Show
          Tim Hunt added a comment - I wrote about the way I do this here: http://tjhunt.blogspot.co.uk/2012/08/automating-git.html
          Hide
          Eloy Lafuente (stronk7) 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.

          TIA and ciao

          Show
          Eloy Lafuente (stronk7) 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. TIA and ciao
          Hide
          Mary Evans added a comment -

          RE-BASED

          Show
          Mary Evans added a comment - RE-BASED
          Hide
          Sam Hemelryk added a comment -

          Thanks Mary + Tim, this has been integrated now.

          Show
          Sam Hemelryk added a comment - Thanks Mary + Tim, this has been integrated now.
          Hide
          Jérôme Mouneyrac added a comment - - edited

          Passing it but the exact same issue can be found few pixel below I'll write an issue for that.

          Show
          Jérôme Mouneyrac added a comment - - edited Passing it but the exact same issue can be found few pixel below I'll write an issue for that.
          Hide
          Mary Evans added a comment -

          In that case we need to list all the themes, like Boxxie, that need tweaking.

          Show
          Mary Evans added a comment - In that case we need to list all the themes, like Boxxie, that need tweaking.
          Hide
          Jérôme Mouneyrac added a comment -

          Tested on master and 2.3. Passed.

          Show
          Jérôme Mouneyrac added a comment - Tested on master and 2.3. Passed.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          A brilliant future is awaiting us out there, better with your code. Let's look towards the future together, this is now closed.

          (and won't be revisiting it unless some regression is found)

          Thanks and ciao

          Show
          Eloy Lafuente (stronk7) added a comment - A brilliant future is awaiting us out there, better with your code. Let's look towards the future together, this is now closed. (and won't be revisiting it unless some regression is found) Thanks and ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: