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
    • Rank:
      33759

      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.

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

        Issue Links

          Activity

          Hide
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          Jason Fowler added a comment -

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

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

          Patch looks good Jason,

          Pushing it for integration review.

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

          rebased

          Show
          Jason Fowler added a comment - rebased
          Hide
          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
          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
          Aparup Banerjee added a comment -

          adding a ping out to lazydaisy too for themes

          Show
          Aparup Banerjee added a comment - adding a ping out to lazydaisy too for themes
          Hide
          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
          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
          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
          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
          Mary Evans added a comment -

          Correct!

          Show
          Mary Evans added a comment - Correct!
          Hide
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          Mary Evans added a comment -

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

          Show
          Mary Evans added a comment - This needs body class editing adding and also needs targeting to completion tracking links only.
          Hide
          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
          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
          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
          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
          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
          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: