Details

    • Testing Instructions:
      Hide

      Please test the following across all the core themes and our supported browsers for visually clarity/contrast etc:

      1. Create a course with activities that have completion enabled
      2. hover over the completion tickbox of the activity
      3. the row for the whole activity should highlight in a light grey
      Show
      Please test the following across all the core themes and our supported browsers for visually clarity/contrast etc: Create a course with activities that have completion enabled hover over the completion tickbox of the activity the row for the whole activity should highlight in a light grey
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      wip-MDL-30818-master

      Description

      The check mark and the item it is associated with can be quite far apart on the screen, thus leading to confusion about which check mark goes with which item. Adding a :hover or similar type highlighting function to each check mark to highlight the entire row would allow users to easily visually associate which check mark goes with each item.

        Gliffy Diagrams

        1. MDL-30818.jpg
          34 kB
        2. themeoops.png
          47 kB

          Issue Links

            Activity

            Hide
            salvetore Michael de Raadt added a comment -

            Thanks for reporting these issues, Greg.

            The more detail we can get in these issues the better. Replication steps and screenshots usually help.

            Show
            salvetore Michael de Raadt added a comment - Thanks for reporting these issues, Greg. The more detail we can get in these issues the better. Replication steps and screenshots usually help.
            Hide
            gdkraus Greg Kraus added a comment -

            This is a quick and dirty example of how to do this. Basically you create a new CSS class like this

            .highlight_completion:hover, .highlight_completion:focus

            { background-color: #eea;}

            Then add that class to each activity and resource present on the main course page that also has an activity completion check box.

            <li class="activity page modtype_page highlight_completion" id="module-38">...</li>

            Show
            gdkraus Greg Kraus added a comment - This is a quick and dirty example of how to do this. Basically you create a new CSS class like this .highlight_completion:hover, .highlight_completion:focus { background-color: #eea;} Then add that class to each activity and resource present on the main course page that also has an activity completion check box. <li class="activity page modtype_page highlight_completion" id="module-38">...</li>
            Hide
            rcollman Chris Collman added a comment -

            I moved the check mark to the left.

            Looked at the base theme, course.css file and these two properties and I changed the inline position from right -20 to left -20. Did not understand the rest so did not mess with it. Here is the code I will put in my custom theme, in the style/course.css file.

            .path-course-view li.activity span.autocompletion,
            .path-course-view li.activity form.togglecompletion

            {display:inline;position:absolute;left:-20px;top:0;z-index:10;padding:0.2em 0;}

            That put the checkmark to the left of the icons.

            Show
            rcollman Chris Collman added a comment - I moved the check mark to the left. Looked at the base theme, course.css file and these two properties and I changed the inline position from right -20 to left -20. Did not understand the rest so did not mess with it. Here is the code I will put in my custom theme, in the style/course.css file. .path-course-view li.activity span.autocompletion, .path-course-view li.activity form.togglecompletion {display:inline;position:absolute;left:-20px;top:0;z-index:10;padding:0.2em 0;} That put the checkmark to the left of the icons.
            Hide
            michaelp Michael Penney added a comment -

            Activity checkbox half in topic sidebar - using leatherbound but this problem happens in any theme with colored topic sidebars.

            Show
            michaelp Michael Penney added a comment - Activity checkbox half in topic sidebar - using leatherbound but this problem happens in any theme with colored topic sidebars.
            Hide
            michaelp Michael Penney added a comment - - edited

            The problem is that the base theme around line 26 of course.css has what appear to be typos:

            .path-course-view li.activity span.autocompletion,
            .path-course-view li.activity form.togglecompletion {display:inline;position:absolute; right:-20px;
            .path-course-view li.activity form.togglecompletion div

            {display:inline;}

            and a few lines down:

            .dir-rtl.path-course-view li.activity span.autocompletion {right:-20px;left:auto;padding:0px;}

            The -20s look like typos to me - what they do is push the completion tickbox off half-way into the right side topic border, which can't have been the developers intention.

            I've attached a screenshot from demo.moodle.org of the problem.

            Show
            michaelp Michael Penney added a comment - - edited The problem is that the base theme around line 26 of course.css has what appear to be typos: .path-course-view li.activity span.autocompletion, .path-course-view li.activity form.togglecompletion {display:inline;position:absolute; right:-20px; .path-course-view li.activity form.togglecompletion div {display:inline;} and a few lines down: .dir-rtl.path-course-view li.activity span.autocompletion {right:-20px; left:auto;padding:0px;} The -20s look like typos to me - what they do is push the completion tickbox off half-way into the right side topic border, which can't have been the developers intention. I've attached a screenshot from demo.moodle.org of the problem.
            Hide
            phalacee Jason Fowler added a comment -

            the -20px's are correct for ltr themes. not sure about the rtl, either way, that would be a different issue to this one.

            Show
            phalacee Jason Fowler added a comment - the -20px's are correct for ltr themes. not sure about the rtl, either way, that would be a different issue to this one.
            Hide
            phalacee Jason Fowler added a comment -

            Greg, I found a way to avoid creating the extra class value, using the existing activity selectors instead.

            Show
            phalacee Jason Fowler added a comment - Greg, I found a way to avoid creating the extra class value, using the existing activity selectors instead.
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Patch looks good Jason,

            Pushing it for integration review.

            Show
            rajeshtaneja Rajesh Taneja added a comment - Patch looks good Jason, Pushing it for integration review.
            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
            phalacee Jason Fowler added a comment -

            rebased

            Show
            phalacee Jason Fowler added a comment - rebased
            Hide
            nebgor Aparup Banerjee added a comment -

            Hi Jason,
            patch seems fine but i'm wondering if the colours need some tweaking across the different themes. Perhaps we should quickly check this across all our core themes at least.

            Show
            nebgor Aparup Banerjee added a comment - Hi Jason, patch seems fine but i'm wondering if the colours need some tweaking across the different themes. Perhaps we should quickly check this across all our core themes at least.
            Hide
            nebgor Aparup Banerjee added a comment -

            adding a ping out to lazydaisy too for themes

            Show
            nebgor Aparup Banerjee added a comment - adding a ping out to lazydaisy too for themes
            Hide
            lazydaisy Mary Evans added a comment - - edited

            Thanks for the PING!

            This is indeed something that could do with changing.

            Not sure how far you have got with this Jason, but would be great if the completion checkbox could be to the left of the title rather than far right as it is at present.

            I have managed to do this using CSS but ended up with layout problems. So if this can be fixed in CORE that would be great. Then there would be no need for highlighting the row in question!

            However, in the meantime adding the hover looks good. Thanks

            Show
            lazydaisy Mary Evans added a comment - - edited Thanks for the PING! This is indeed something that could do with changing. Not sure how far you have got with this Jason, but would be great if the completion checkbox could be to the left of the title rather than far right as it is at present. I have managed to do this using CSS but ended up with layout problems. So if this can be fixed in CORE that would be great. Then there would be no need for highlighting the row in question! However, in the meantime adding the hover looks good. Thanks
            Hide
            phalacee Jason Fowler added a comment -

            Mary seems happy with the colour, and really, each theme could be adjusted to use it's own colour, right?

            Show
            phalacee Jason Fowler added a comment - Mary seems happy with the colour, and really, each theme could be adjusted to use it's own colour, right?
            Hide
            lazydaisy Mary Evans added a comment -

            Correct!

            Show
            lazydaisy Mary Evans added a comment - Correct!
            Hide
            nebgor Aparup Banerjee added a comment -

            cool, this has been integrated into master now.

            Jason: i'm adding to tests, to test across all core themes visually (and across browsers). if core themes break visually it'd need to be fixed before weekly rolls. (hence my question about whether other themes were tested or not)

            Show
            nebgor Aparup Banerjee added a comment - cool, this has been integrated into master now. Jason: i'm adding to tests, to test across all core themes visually (and across browsers). if core themes break visually it'd need to be fixed before weekly rolls. (hence my question about whether other themes were tested or not)
            Hide
            lazydaisy Mary Evans added a comment - - edited

            @Aparup
            @Jason

            I can verify that ALL themes work well with this fix.

            My last good deed for the day!

            Good night all

            EDIT:

            I have just tested FF/Opera/Chrome/IE9 and these work OK. I don't have Safari, but I do not envisage any problem with the hover. If it works in IE it should work everywhere else! LOL

            Show
            lazydaisy Mary Evans added a comment - - edited @Aparup @Jason I can verify that ALL themes work well with this fix. My last good deed for the day! Good night all EDIT: I have just tested FF/Opera/Chrome/IE9 and these work OK. I don't have Safari, but I do not envisage any problem with the hover. If it works in IE it should work everywhere else! LOL
            Hide
            salvetore Michael de Raadt added a comment -

            MDL-31492 could probably closed if this is integrated. It's your issue Apu, so I'll leave that to you.

            Show
            salvetore Michael de Raadt added a comment - MDL-31492 could probably closed if this is integrated. It's your issue Apu, so I'll leave that to you.
            Hide
            salvetore Michael de Raadt added a comment -

            We could also suggest, to the reporters of MDL-31603 and MDL-34729 that this could satisfy their desires (expressed as moving the completion icon). I think this solution is slightly superior as it accommodates both LTR and RTL presentations.

            Show
            salvetore Michael de Raadt added a comment - We could also suggest, to the reporters of MDL-31603 and MDL-34729 that this could satisfy their desires (expressed as moving the completion icon). I think this solution is slightly superior as it accommodates both LTR and RTL presentations.
            Hide
            nebgor Aparup Banerjee added a comment -

            heh i think its safe to say Mary is the tester for this issue and it has been passed, Thanks Mary!

            Show
            nebgor Aparup Banerjee added a comment - heh i think its safe to say Mary is the tester for this issue and it has been passed, Thanks Mary!
            Hide
            nebgor Aparup Banerjee added a comment -

            Thanks MdR, the screenshot there looks to be the exact (albeit a lighter shade of grey) result of this fix! i've closed it now.

            Show
            nebgor Aparup Banerjee added a comment - Thanks MdR, the screenshot there looks to be the exact (albeit a lighter shade of grey) result of this fix! i've closed it now.
            Hide
            lazydaisy Mary Evans added a comment -

            This is what the fix is doing. Michael's screen-shot seems to be just highlighting the check-box.

            Show
            lazydaisy Mary Evans added a comment - This is what the fix is doing. Michael's screen-shot seems to be just highlighting the check-box.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Gutta cavat lapidem, non vi sed saepe cadendo - Ovidio

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

            Thanks!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Gutta cavat lapidem, non vi sed saepe cadendo - Ovidio This issue has been integrated upstream and is now available both via git and cvs (and in some hours, via mirrors and downloads). Thanks!
            Hide
            poltawski Dan Poltawski added a comment -

            Some people hate this change and want it reversed - see MDL-42113. Please can involved parties look at this issue to ensure we get a solution which works for everyone.

            Show
            poltawski Dan Poltawski added a comment - Some people hate this change and want it reversed - see MDL-42113 . Please can involved parties look at this issue to ensure we get a solution which works for everyone.
            Hide
            lazydaisy Mary Evans added a comment -

            This needs body class editing adding and also needs targeting to completion tracking links only.

            Show
            lazydaisy Mary Evans added a comment - This needs body class editing adding and also needs targeting to completion tracking links only.
            Hide
            quen Sam Marshall added a comment -

            Mary: It sounds like this change was intended for students/ordinary users, not for editing, so adding the editing class would not help?

            Without wanting to throw a spanner in the works, I'll point out MDL-34729 and MDL-31603 (already linked to this) as other potential ways of solving the spot-the-icon problem - by moving completion tracking icons to the left instead of adding hover effects. Not saying I am necessarily in favour of those, because to be honest, I'm not sure what the best solution is!

            Show
            quen Sam Marshall added a comment - Mary: It sounds like this change was intended for students/ordinary users, not for editing, so adding the editing class would not help? Without wanting to throw a spanner in the works, I'll point out MDL-34729 and MDL-31603 (already linked to this) as other potential ways of solving the spot-the-icon problem - by moving completion tracking icons to the left instead of adding hover effects. Not saying I am necessarily in favour of those, because to be honest, I'm not sure what the best solution is!
            Hide
            lazydaisy Mary Evans added a comment -

            Perhaps it's time to rethink how the individual activity elements are styled, as I am finding all kinds of inconsistencies in this area, which in turn, create different views because of the class styles that are being used. Also, why is it not possible to add a completion tracker icon in the same area as the editing icons?

            Show
            lazydaisy Mary Evans added a comment - Perhaps it's time to rethink how the individual activity elements are styled, as I am finding all kinds of inconsistencies in this area, which in turn, create different views because of the class styles that are being used. Also, why is it not possible to add a completion tracker icon in the same area as the editing icons?
            Hide
            phalacee Jason Fowler added a comment -

            +1 for a better approach to the problem. No idea what it would be, hence me doing it this way in the first place.

            Show
            phalacee Jason Fowler added a comment - +1 for a better approach to the problem. No idea what it would be, hence me doing it this way in the first place.

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  3/Dec/12