Moodle
  1. Moodle
  2. MDL-37771

Edit Quiz: background color sometimes hides the text as the text color is similar

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3.4, 2.4.1
    • Fix Version/s: 2.3.6, 2.4.3
    • Component/s: Quiz, Themes
    • Labels:
    • Testing Instructions:
      Hide
      1. Clear all caches
      2. Go to the 'Edit quiz' page for any quiz.
      3. Make sure that the question bank contents are visible. (If not, click [Show].)
      4. Make sure there are several questions shown in the question bank. (If not, add some.)
      5. Now look at the page in every theme, and make sure the styling of the question bank display is OK.
      Show
      Clear all caches Go to the 'Edit quiz' page for any quiz. Make sure that the question bank contents are visible. (If not, click [Show] .) Make sure there are several questions shown in the question bank. (If not, add some.) Now look at the page in every theme, and make sure the styling of the question bank display is OK.
    • Affected Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull Master Branch:

      Description

      This bug was found during testing MDL-29723

      See screen capture. Need to check all themes.

        Gliffy Diagrams

        1. MDL-37771.jpg
          78 kB
        2. quiz.png
          371 kB
        3. Screenshot_30_01_13_12_03_PM.png
          179 kB
        4. Screenshot_30_01_13_12_20_PM.png
          162 kB

          Issue Links

            Activity

            Hide
            Tim Hunt added a comment -

            Mary, do you want to look at this, or shall I? Basically, if you want, feel free to assign this to yourself.

            Show
            Tim Hunt added a comment - Mary, do you want to look at this, or shall I? Basically, if you want, feel free to assign this to yourself.
            Hide
            Tim Hunt added a comment -

            OK, Mary, this seems to fix it for me.

            I explained the changes I made in the commit comment. https://github.com/timhunt/moodle/commit/9d9de8437cd55fcdc3e1d07df986ff66abfa9af8

            Please could you take a look and let me know what you think.

            Show
            Tim Hunt added a comment - OK, Mary, this seems to fix it for me. I explained the changes I made in the commit comment. https://github.com/timhunt/moodle/commit/9d9de8437cd55fcdc3e1d07df986ff66abfa9af8 Please could you take a look and let me know what you think.
            Hide
            Dan Poltawski added a comment -

            Hi Tim,

            I know you were after Mary reviewing this but she hasn't seem to be able to yet so I took a look.

            I think your solution makes sense. It would be great to have Mary confirm her support for it too. So if required please assign her as peer reviewer.

            Show
            Dan Poltawski added a comment - Hi Tim, I know you were after Mary reviewing this but she hasn't seem to be able to yet so I took a look. I think your solution makes sense. It would be great to have Mary confirm her support for it too. So if required please assign her as peer reviewer.
            Hide
            Tim Hunt added a comment -

            Looking again at the fix, it still makes sense to me, so submitting for integration.

            I still would like it if Mary could give it the OK too, but she has a lot of other bugs, so I won't wait.

            Show
            Tim Hunt added a comment - Looking again at the fix, it still makes sense to me, so submitting for integration. I still would like it if Mary could give it the OK too, but she has a lot of other bugs, so I won't wait.
            Hide
            Mary Evans added a comment -

            I don't know how I missed this!
            You should have sent me a message.

            I'll take a look now.

            Show
            Mary Evans added a comment - I don't know how I missed this! You should have sent me a message. I'll take a look now.
            Hide
            Mary Evans added a comment - - edited

            This should have been assigned to me as it's a Theme BUG. However, the problem stems from the class header.

            This would be my way of dealing with this:

            /* Question Bank
            -------------------------*/
             
            #page-mod-quiz-edit .questionbankwindow.block div.header,
            #page-mod-quiz-edit table#categoryquestions thead .header{
                background-color: #69804e;
            }
             
            #page-mod-quiz-edit table#categoryquestions thead .header,
            #page-mod-quiz-edit table#categoryquestions thead .header a:link,
            #page-mod-quiz-edit table#categoryquestions thead .header a:visited {
                color: #fff;
            }
             
            #page-mod-quiz-edit table#categoryquestions thead .header a:hover {
                text-decoration: underline;
            }
            

            Show
            Mary Evans added a comment - - edited This should have been assigned to me as it's a Theme BUG. However, the problem stems from the class header. This would be my way of dealing with this: /* Question Bank -------------------------*/   #page-mod-quiz-edit .questionbankwindow.block div.header, #page-mod-quiz-edit table#categoryquestions thead .header{ background-color: #69804e; }   #page-mod-quiz-edit table#categoryquestions thead .header, #page-mod-quiz-edit table#categoryquestions thead .header a:link, #page-mod-quiz-edit table#categoryquestions thead .header a:visited { color: #fff; }   #page-mod-quiz-edit table#categoryquestions thead .header a:hover { text-decoration: underline; }
            Hide
            Tim Hunt added a comment -

            Your proposal would fix this specific instance of the problem, but would not fix other problems caused by the over-broad .header rules that are ambiguous.

            Up to you, do you wan to make a fix your way, or shall I submit mine for integration?

            Show
            Tim Hunt added a comment - Your proposal would fix this specific instance of the problem, but would not fix other problems caused by the over-broad .header rules that are ambiguous. Up to you, do you wan to make a fix your way, or shall I submit mine for integration?
            Hide
            Mary Evans added a comment - - edited

            In actual fact you could add the above fix changeing the background-color to dark blue to match the Question Bank header, and make the hover white too. Like so...

            /* Question Bank
            -------------------------*/
             
            #page-mod-quiz-edit .questionbankwindow.block div.header,
            #page-mod-quiz-edit table#categoryquestions thead .header{
                background-color: #009;
            }
             
            #page-mod-quiz-edit table#categoryquestions thead .header,
            #page-mod-quiz-edit table#categoryquestions thead .header a:link,
            #page-mod-quiz-edit table#categoryquestions thead .header a:visited,
            #page-mod-quiz-edit table#categoryquestions thead .header a:hover {
                color: #fff;
            }
            #page-mod-quiz-edit table#categoryquestions thead .header a:hover {
                text-decoration: underline;
            }
            

            Which would (or should) fix all themes.

            Show
            Mary Evans added a comment - - edited In actual fact you could add the above fix changeing the background-color to dark blue to match the Question Bank header, and make the hover white too. Like so... /* Question Bank -------------------------*/   #page-mod-quiz-edit .questionbankwindow.block div.header, #page-mod-quiz-edit table#categoryquestions thead .header{ background-color: #009; }   #page-mod-quiz-edit table#categoryquestions thead .header, #page-mod-quiz-edit table#categoryquestions thead .header a:link, #page-mod-quiz-edit table#categoryquestions thead .header a:visited, #page-mod-quiz-edit table#categoryquestions thead .header a:hover { color: #fff; } #page-mod-quiz-edit table#categoryquestions thead .header a:hover { text-decoration: underline; } Which would (or should) fix all themes.
            Hide
            Tim Hunt added a comment -

            I think the dark blue is a weird anomaly. If anything we should be getting rid of that, not adding more of it.

            Show
            Tim Hunt added a comment - I think the dark blue is a weird anomaly. If anything we should be getting rid of that, not adding more of it.
            Hide
            Mary Evans added a comment -

            It looks OK in the attached file as it would look in Boxxie, if the CSS went into Base theme.

            Show
            Mary Evans added a comment - It looks OK in the attached file as it would look in Boxxie, if the CSS went into Base theme.
            Hide
            Tim Hunt added a comment -

            You are right. That looks better. Feel free to submit that for integration. Thanks.

            Show
            Tim Hunt added a comment - You are right. That looks better. Feel free to submit that for integration. Thanks.
            Hide
            Mary Evans added a comment -

            Will do.
            I presume you mean for me to take over here?
            I've just assigned it to myself and will do the GIT stuff later.

            Show
            Mary Evans added a comment - Will do. I presume you mean for me to take over here? I've just assigned it to myself and will do the GIT stuff later.
            Hide
            Mary Evans added a comment - - edited

            By the way...I noticed that the tiny arrow that indicates the sort direction, tends to switch sides every so often. Have you noticed that? You can see in the image I uploaded that it's on the right, normally its on the left.

            Show
            Mary Evans added a comment - - edited By the way...I noticed that the tiny arrow that indicates the sort direction, tends to switch sides every so often. Have you noticed that? You can see in the image I uploaded that it's on the right, normally its on the left.
            Hide
            Tim Hunt added a comment -

            Great! Thank you very much Mary. I will try to do you a quick peer-review after you have done your git thing.

            Presumably this should not be 'waiting for integration review' yet?

            Show
            Tim Hunt added a comment - Great! Thank you very much Mary. I will try to do you a quick peer-review after you have done your git thing. Presumably this should not be 'waiting for integration review' yet?
            Hide
            Mary Evans added a comment -

            Reopening to start development

            Show
            Mary Evans added a comment - Reopening to start development
            Hide
            Mary Evans added a comment -

            Tim can you Peer Review this in Master? Cheers

            Show
            Mary Evans added a comment - Tim can you Peer Review this in Master? Cheers
            Hide
            Tim Hunt added a comment -

            The only problem is the comment, These are far from the only rules affecting the question bank, aren't they?

            Show
            Tim Hunt added a comment - The only problem is the comment, These are far from the only rules affecting the question bank, aren't they?
            Hide
            Mary Evans added a comment -

            The commit comment or the one in the stylesheet.

            Show
            Mary Evans added a comment - The commit comment or the one in the stylesheet.
            Hide
            Tim Hunt added a comment -

            Sorry not to be clear, I am looking at what I see when I click the three "Pull ... Diff URL" links above.

            1. I just noticed that the master branch is different from the other two. The

            #page-mod-quiz-edit table#categoryquestions thead .header a:hover {

            line is missing.

            2. My concern is that the new rules you added are specific to the quiz, shouldn't they be in mod/quiz/styles.php instead? I think there are already some other similar rules around https://github.com/lazydaisy/moodle/blob/5803020eba191e37046a2bf8fd5891d4c742a1d2/mod/quiz/styles.css#L283.

            3. The other problem with the comment is that I have not seen that style of commenting used anywhere else in Moodle. For example, look at the other comments in theme/base/style/question.css or mod/quiz/styles.css

            Show
            Tim Hunt added a comment - Sorry not to be clear, I am looking at what I see when I click the three "Pull ... Diff URL" links above. 1. I just noticed that the master branch is different from the other two. The #page-mod-quiz-edit table#categoryquestions thead .header a:hover { line is missing. 2. My concern is that the new rules you added are specific to the quiz, shouldn't they be in mod/quiz/styles.php instead? I think there are already some other similar rules around https://github.com/lazydaisy/moodle/blob/5803020eba191e37046a2bf8fd5891d4c742a1d2/mod/quiz/styles.css#L283 . 3. The other problem with the comment is that I have not seen that style of commenting used anywhere else in Moodle. For example, look at the other comments in theme/base/style/question.css or mod/quiz/styles.css
            Hide
            Mary Evans added a comment -

            Ah, I see we have been talking at cross purposes.

            I had assumed you wanted me to add this to base as I had suggested so that the style of the Q.B.W whould be the same across the board, as far as themes were concerned. That way it's easier to override.

            However to keep thing simple I'll add them to the mod/quiz/styles.css - no problem.

            As for the comment in the css. That is a style I have used for a long time and you are the first to comment, so this shows you are perhaps more observant than most!

            I like that style as it is easier for me to see the different headings
            For example:

            /**generalbox**/
            .generalbox {border: 1px solid #ddd}

            is not as easy to see in a sea of css
            as the comment style which I prefer below

            /* generalbox
            -----------------*/
            .generalbox {border: 1px solid #ddd}

            Show
            Mary Evans added a comment - Ah, I see we have been talking at cross purposes. I had assumed you wanted me to add this to base as I had suggested so that the style of the Q.B.W whould be the same across the board, as far as themes were concerned. That way it's easier to override. However to keep thing simple I'll add them to the mod/quiz/styles.css - no problem. As for the comment in the css. That is a style I have used for a long time and you are the first to comment, so this shows you are perhaps more observant than most! I like that style as it is easier for me to see the different headings For example: /**generalbox**/ .generalbox {border: 1px solid #ddd} is not as easy to see in a sea of css as the comment style which I prefer below /* generalbox -----------------*/ .generalbox {border: 1px solid #ddd}
            Hide
            Tim Hunt added a comment -

            Comment style is really up to the integrators. If they are happy with your style, that is OK.

            The strange dark blue colour is only used on the quiz edit page, so I think it is OK to just apply these styles in the quiz.

            Is guess this is about ready for integration.

            Show
            Tim Hunt added a comment - Comment style is really up to the integrators. If they are happy with your style, that is OK. The strange dark blue colour is only used on the quiz edit page, so I think it is OK to just apply these styles in the quiz. Is guess this is about ready for integration.
            Hide
            Tim Hunt added a comment -

            Sorry, you are probably about ready to kill me at this point, but I have just remembered MDL-38209. If you submit this for integration this week, we will get horrible merge conflicts. See https://github.com/timhunt/moodle/compare/master...MDL-38209 - I basically completely re-wrote mod/quiz/styles.css to fix the coding style.

            Show
            Tim Hunt added a comment - Sorry, you are probably about ready to kill me at this point, but I have just remembered MDL-38209 . If you submit this for integration this week, we will get horrible merge conflicts. See https://github.com/timhunt/moodle/compare/master...MDL-38209 - I basically completely re-wrote mod/quiz/styles.css to fix the coding style.
            Hide
            Mary Evans added a comment -

            Don't worry about it. I'm in a similar conflict with some MyMobile theme changes. This just adds to the fun of it all.

            Good job you said, I have only done the master branch so I can delete that and wait until MDL-38209 is integrated.

            Have you re-based MDL-38209 yet? I don't think you will have any problems with it.

            Show
            Mary Evans added a comment - Don't worry about it. I'm in a similar conflict with some MyMobile theme changes. This just adds to the fun of it all. Good job you said, I have only done the master branch so I can delete that and wait until MDL-38209 is integrated. Have you re-based MDL-38209 yet? I don't think you will have any problems with it.
            Hide
            Tim Hunt added a comment -

            Yes, I rebased it, and did have a problem with a commit of Fred's from last week.

            Show
            Tim Hunt added a comment - Yes, I rebased it, and did have a problem with a commit of Fred's from last week.
            Hide
            Mary Evans added a comment -

            MDL-38209 Passed Testing!

            So I can fix this now when Moodle has been updated...yes?

            Show
            Mary Evans added a comment - MDL-38209 Passed Testing! So I can fix this now when Moodle has been updated...yes?
            Hide
            Mary Evans added a comment -

            Tim,

            I have just been testing this as I am working on it and it's not working. In fact it is creating more problems in themes!

            It would be better just to adjust each theme, as I would end up having to do that anyway if I changed this in quiz/style.css.

            Show
            Mary Evans added a comment - Tim, I have just been testing this as I am working on it and it's not working. In fact it is creating more problems in themes! It would be better just to adjust each theme, as I would end up having to do that anyway if I changed this in quiz/style.css.
            Hide
            Tim Hunt added a comment -

            I am happy with this if you are.

            Show
            Tim Hunt added a comment - I am happy with this if you are.
            Hide
            Mary Evans added a comment - - edited

            As you will have seen I've done the fixes and submitted them for integration.
            I've just added one tiny change as a new commit to fix a typo. Do you think that will be OK or will I need to squash/merge them?

            Show
            Mary Evans added a comment - - edited As you will have seen I've done the fixes and submitted them for integration. I've just added one tiny change as a new commit to fix a typo. Do you think that will be OK or will I need to squash/merge them?
            Hide
            Tim Hunt added a comment -

            I think you should squash the commits. Thanks.

            Show
            Tim Hunt added a comment - I think you should squash the commits. Thanks.
            Hide
            Mary Evans added a comment -

            Done!

            Of all the themes in these fixes Serenity looks the best.

            Show
            Mary Evans added a comment - Done! Of all the themes in these fixes Serenity looks the best.
            Hide
            Damyon Wiese added a comment -

            Thanks Mary and Tim,

            This is integrated now on master, 24 and 23. Just a note: I had to amend the git commit message on master as it was missing the blank line after the first line. This is important because lots of git commands are expecting it (like rebase).

            I added a step to the testing instructions to purge caches as I didn't add a version bump for this.

            Show
            Damyon Wiese added a comment - Thanks Mary and Tim, This is integrated now on master, 24 and 23. Just a note: I had to amend the git commit message on master as it was missing the blank line after the first line. This is important because lots of git commands are expecting it (like rebase). I added a step to the testing instructions to purge caches as I didn't add a version bump for this.
            Hide
            Rajesh Taneja added a comment -

            Thanks Tim and Mary,

            Most themes doesn't have background color in question, except magazine. But text looks fine (white over green)

            FYI: It seems brick theme is not rendering question bank properly (See attached image), and it's same in stable.

            Show
            Rajesh Taneja added a comment - Thanks Tim and Mary, Most themes doesn't have background color in question, except magazine. But text looks fine (white over green) FYI: It seems brick theme is not rendering question bank properly (See attached image), and it's same in stable.
            Hide
            Eloy Lafuente (stronk7) added a comment -

            This issue has been integrated upstream and is now available both via git and cvs (and in some hours, via mirrors and downloads).

            Thanks!

            PS: Yay, legacy template messages. Yes, you're ok, we don't have CVS anymore!

            Show
            Eloy Lafuente (stronk7) added a comment - This issue has been integrated upstream and is now available both via git and cvs (and in some hours, via mirrors and downloads). Thanks! PS: Yay, legacy template messages. Yes, you're ok, we don't have CVS anymore!

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved: