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

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

    Details

    • Type: Bug
    • Status: Closed
    • Priority: 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

          Attachments

            Issue Links

              Activity

              Hide
              timhunt 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
              timhunt 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
              timhunt 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
              timhunt 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
              poltawski 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
              poltawski 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
              timhunt 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
              timhunt 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
              lazydaisy 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
              lazydaisy 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
              lazydaisy 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
              lazydaisy 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
              timhunt 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
              timhunt 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
              lazydaisy 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
              lazydaisy 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
              timhunt 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
              timhunt 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
              lazydaisy 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
              lazydaisy 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
              timhunt Tim Hunt added a comment -

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

              Show
              timhunt Tim Hunt added a comment - You are right. That looks better. Feel free to submit that for integration. Thanks.
              Hide
              lazydaisy 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
              lazydaisy 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
              lazydaisy 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
              lazydaisy 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
              timhunt 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
              timhunt 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
              lazydaisy Mary Evans added a comment -

              Reopening to start development

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

              Tim can you Peer Review this in Master? Cheers

              Show
              lazydaisy Mary Evans added a comment - Tim can you Peer Review this in Master? Cheers
              Hide
              timhunt 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
              timhunt 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
              lazydaisy Mary Evans added a comment -

              The commit comment or the one in the stylesheet.

              Show
              lazydaisy Mary Evans added a comment - The commit comment or the one in the stylesheet.
              Hide
              timhunt 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
              timhunt 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
              lazydaisy 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
              lazydaisy 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
              timhunt 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
              timhunt 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
              timhunt 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
              timhunt 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
              lazydaisy 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
              lazydaisy 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
              timhunt Tim Hunt added a comment -

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

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

              MDL-38209 Passed Testing!

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

              Show
              lazydaisy Mary Evans added a comment - MDL-38209 Passed Testing! So I can fix this now when Moodle has been updated...yes?
              Hide
              lazydaisy 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
              lazydaisy 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
              timhunt Tim Hunt added a comment -

              I am happy with this if you are.

              Show
              timhunt Tim Hunt added a comment - I am happy with this if you are.
              Hide
              lazydaisy 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
              lazydaisy 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
              timhunt Tim Hunt added a comment -

              I think you should squash the commits. Thanks.

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

              Done!

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

              Show
              lazydaisy Mary Evans added a comment - Done! Of all the themes in these fixes Serenity looks the best.
              Hide
              damyon 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 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
              rajeshtaneja 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
              rajeshtaneja 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
              stronk7 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
              stronk7 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:
                    Fix Release Date:
                    18/Mar/13