Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-29723

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

    Details

    • Type: Bug
    • Status: Closed
    • Priority: 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 Master Branch:
      wip-MDL-29723_master

      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).

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              timhunt Tim Hunt added a comment -

              Leatherbound theme issue. Re-assigning.

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

              Hi Tim,

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

              Show
              lazydaisy Mary Evans added a comment - Hi Tim, If I find this is a QUIZ style issue can I change the stylesheet in QUIZ?
              Hide
              lazydaisy 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
              lazydaisy 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
              lazydaisy Mary Evans added a comment -

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

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

              Attached image showing changes.

              Show
              lazydaisy Mary Evans added a comment - Attached image showing changes.
              Hide
              timhunt 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
              timhunt 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
              lazydaisy 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
              lazydaisy 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
              lazydaisy Mary Evans added a comment -

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

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

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

              Show
              timhunt Tim Hunt added a comment - That makes sense. Thanks. Clearly I need to look more closely, which is will do tomorrow.
              Hide
              timhunt 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
              timhunt 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
              lazydaisy 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
              lazydaisy 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
              timhunt 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
              timhunt 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
              lazydaisy 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
              lazydaisy 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
              timhunt 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
              timhunt 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
              lazydaisy 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
              lazydaisy 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
              timhunt Tim Hunt added a comment -

              Now works for me! I have chat open.

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

              Tim,

              What did we decide abut this, can you remember?

              Show
              lazydaisy Mary Evans added a comment - Tim, What did we decide abut this, can you remember?
              Hide
              timhunt 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
              timhunt 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
              lazydaisy 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
              lazydaisy 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
              timhunt Tim Hunt added a comment -

              Sorry, I was not paying enough attention.

              Show
              timhunt Tim Hunt added a comment - Sorry, I was not paying enough attention.
              Hide
              lazydaisy 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
              lazydaisy 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
              lazydaisy 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
              lazydaisy 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
              timhunt 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
              timhunt 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
              lazydaisy 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
              lazydaisy 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
              lazydaisy Mary Evans added a comment -

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

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

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

              Show
              timhunt Tim Hunt added a comment - Yay! Thanks. +1 from me as peer-reviewer.
              Hide
              lazydaisy 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
              lazydaisy 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
              timhunt 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
              timhunt 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
              lazydaisy 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
              lazydaisy 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
              timhunt 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
              timhunt 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
              stronk7 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
              stronk7 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
              lazydaisy Mary Evans added a comment -

              RE-BASED

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

              Thanks Mary + Tim, this has been integrated now.

              Show
              samhemelryk Sam Hemelryk added a comment - Thanks Mary + Tim, this has been integrated now.
              Hide
              jerome 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
              jerome 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
              lazydaisy Mary Evans added a comment -

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

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

              Tested on master and 2.3. Passed.

              Show
              jerome Jérôme Mouneyrac added a comment - Tested on master and 2.3. Passed.
              Hide
              stronk7 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
              stronk7 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:
                    Fix Release Date:
                    11/Mar/13