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

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.

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.

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.

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

I'll take a look now.

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;
}

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?

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.

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

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

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

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.

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.

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?

Reopening to start development

Tim can you Peer Review this in Master? Cheers

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

The commit comment or the one in the stylesheet.

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

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}

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.

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.

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.

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

MDL-38209 Passed Testing!

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

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.

I am happy with this if you are.

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?

I think you should squash the commits. Thanks.

Done!

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

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.

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.

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!