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

          Attachments

            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