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

          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