Details

    • Affected Branches:
      MOODLE_18_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE
    • Rank:
      34288

      Description

      Many school users find the icons for activities and block menus unintuitive as they can't click them - there is not a link surrouding the images in the html.

      This patch adds a new variable $CFG->clickable_icons, which allows many of the unclickable icons to be clickable - whilst allowing the previous behaviour to be kept.

      When $CFG->clickable_icons is switched on, course activity icons are clickable, as are the course category icons in the all courses. As well as all block items which are links with icons.

      Enabiling activity icons and course categories to be clickable is a fairly simple change to the code which displays them.

      In order to make block links have clickable icons the code is slightly more hacky - if clickable icons are switched on a regular expression checks each block item with an icon to see if it starts with a link, extracts the link tag and puts it around the icon. I am not really sure if this could be done better with the way that moodle currently displays block items.

      Dan.

      1. 20091013_clickableicons.patch
        3 kB
        Rossiani Wijaya
      2. 20091214_mdl_6820.patch
        1 kB
        Rossiani Wijaya
      3. 20100819_MDL-6820_2.0.patch
        9 kB
        Rossiani Wijaya
      4. clickable_links.patch
        4 kB
        Dan Poltawski
      5. clickable_settings.patch
        1 kB
        Dan Poltawski

        Issue Links

          Activity

          Hide
          Dan Poltawski added a comment -

          Here is a patch which adds the setting to the appeance settings which I forgot to add. (with poorly worded language strings.. )

          Show
          Dan Poltawski added a comment - Here is a patch which adds the setting to the appeance settings which I forgot to add. (with poorly worded language strings.. )
          Hide
          Julian Ridden added a comment -

          I think this is a great idea. Consider it 'voted'.

          Show
          Julian Ridden added a comment - I think this is a great idea. Consider it 'voted'.
          Hide
          Anthony Borrow added a comment -

          I added item #3 to http://docs.moodle.org/en/Development:Interface_guidelines#Standard_navigation_tools which states:

          Links should include associated image (if available). For example, looking at an assignment in a course displays the assignment pic and then the assignment description as a single link; however, most of the blocks like the activity block exclude the image from the link. This is important as some folks are more graphically rather than text oriented and click on the picture and do not understand why it is not working. We need to be consistent.

          I'm not sure why this needs to be an option. At one point I mentioned this in a chat with Martin and he felt that the image should be part of the link (as I recall). I frequently watch users click on the icon and then get confused why it did not work. My +1 to get this patched up ASAP.

          Peace - Anthony

          Show
          Anthony Borrow added a comment - I added item #3 to http://docs.moodle.org/en/Development:Interface_guidelines#Standard_navigation_tools which states: Links should include associated image (if available). For example, looking at an assignment in a course displays the assignment pic and then the assignment description as a single link; however, most of the blocks like the activity block exclude the image from the link. This is important as some folks are more graphically rather than text oriented and click on the picture and do not understand why it is not working. We need to be consistent. I'm not sure why this needs to be an option. At one point I mentioned this in a chat with Martin and he felt that the image should be part of the link (as I recall). I frequently watch users click on the icon and then get confused why it did not work. My +1 to get this patched up ASAP. Peace - Anthony
          Hide
          Olli Savolainen added a comment -

          I agree that this should be decided in one direction or another - it should not be up to single sysadmins to make usability decisions of this level of precision.

          Show
          Olli Savolainen added a comment - I agree that this should be decided in one direction or another - it should not be up to single sysadmins to make usability decisions of this level of precision.
          Hide
          Martin Dougiamas added a comment -

          Rosie can you check this patch and make sure it works?

          I can help you check it in to 1.9 and HEAD then.

          Show
          Martin Dougiamas added a comment - Rosie can you check this patch and make sure it works? I can help you check it in to 1.9 and HEAD then.
          Hide
          Rossiani Wijaya added a comment -

          Martin,

          the clickable_links.patch can't be apply to 1.9.6 version. I will create a new patch for this issue.

          Rosie

          Show
          Rossiani Wijaya added a comment - Martin, the clickable_links.patch can't be apply to 1.9.6 version. I will create a new patch for this issue. Rosie
          Hide
          Anthony Borrow added a comment -

          Thanks Rosie for following up on the clickable icons as I think this will be a big improvement for UI. Let me know if you need any help testing. Peace - Anthony

          Show
          Anthony Borrow added a comment - Thanks Rosie for following up on the clickable icons as I think this will be a big improvement for UI. Let me know if you need any help testing. Peace - Anthony
          Hide
          Martin Dougiamas added a comment -

          Yeah my +1 to:

          • not have a setting for it, just turn it on all the time and
          • include the image and text in one link (two links is bad for accessibility)
          Show
          Martin Dougiamas added a comment - Yeah my +1 to: not have a setting for it, just turn it on all the time and include the image and text in one link (two links is bad for accessibility)
          Hide
          Dan Poltawski added a comment -

          Hopefully that means we can get rid of the regular expression search and replace to links (which I hate from my original 3 year old patch )

          Show
          Dan Poltawski added a comment - Hopefully that means we can get rid of the regular expression search and replace to links (which I hate from my original 3 year old patch )
          Hide
          Anthony Borrow added a comment -

          I may be confusing this with another issue; however, I thought that there was a function in the library that generated most of the links and that it was a pretty simple patch. If I get a chance I will go back and review and see if I can find what I only vaguely remember at the moment. Peace - Anthony

          Show
          Anthony Borrow added a comment - I may be confusing this with another issue; however, I thought that there was a function in the library that generated most of the links and that it was a pretty simple patch. If I get a chance I will go back and review and see if I can find what I only vaguely remember at the moment. Peace - Anthony
          Hide
          Rossiani Wijaya added a comment -

          Attached is the new patch for version 1.9.6. Please let me know if there's more issue related to the patch.

          Thank you
          Rosie

          Show
          Rossiani Wijaya added a comment - Attached is the new patch for version 1.9.6. Please let me know if there's more issue related to the patch. Thank you Rosie
          Hide
          Anthony Borrow added a comment -

          Rosie - I applied the patch and it looks good to me. This is a significant UI improvement as I, like Dan, frequently watch users click on the icon and not understand why nothing was happening. Thanks for your work on this! Peace - Anthony

          Show
          Anthony Borrow added a comment - Rosie - I applied the patch and it looks good to me. This is a significant UI improvement as I, like Dan, frequently watch users click on the icon and not understand why nothing was happening. Thanks for your work on this! Peace - Anthony
          Hide
          Rossiani Wijaya added a comment -

          Great!

          @Anthony: Thank you for testing it out.

          ~Rosie

          Show
          Rossiani Wijaya added a comment - Great! @Anthony: Thank you for testing it out. ~Rosie
          Hide
          Martin Dougiamas added a comment -

          I've just been testing this too:

          1) Note that the links to activities in sections were already fixed in CVS a long time ago.

          2) On the course listing, the fixes to category icons result in formatting changes which I can't accept .... the tables there are very delicate. I think we need to solve this properly in HEAD more like this: http://docs.moodle.org/en/User:Frank_Ralf/Semantic_HTML4

          3) Yeah, that parsing function that pulls the link to bits should not be necessary ... I think it's better to insert the icon inside the link text using a simple search/replace. The problem will be adjusting the HTML from two divs down to one while retaining formatting and NOT breaking backward compatibility for themes.

          Sorry but we'll need to defer this past 1.9.6.

          Show
          Martin Dougiamas added a comment - I've just been testing this too: 1) Note that the links to activities in sections were already fixed in CVS a long time ago. 2) On the course listing, the fixes to category icons result in formatting changes which I can't accept .... the tables there are very delicate. I think we need to solve this properly in HEAD more like this: http://docs.moodle.org/en/User:Frank_Ralf/Semantic_HTML4 3) Yeah, that parsing function that pulls the link to bits should not be necessary ... I think it's better to insert the icon inside the link text using a simple search/replace. The problem will be adjusting the HTML from two divs down to one while retaining formatting and NOT breaking backward compatibility for themes. Sorry but we'll need to defer this past 1.9.6.
          Hide
          Martin Dougiamas added a comment -

          A (faster) patch to fix the blocks with icons ... it works for me but not sure about regressions (themes etc) so not sure about 1.9.6 ...

           
          Index: weblib.php
          ===================================================================
          RCS file: /cvsroot/moodle/moodle/lib/weblib.php,v
          retrieving revision 1.970.2.144
          diff -r1.970.2.144 weblib.php
          6434,6435c6434,6440
          <                 if ($icons) {
          <                     echo '<div class="icon column c0">'. $icons[$key] .'</div>';
          ---
          >                 if ($icons) {  // Hacky way to insert the icon into the link.  See MDL-6820
          >                     $newstring = str_replace('">', '">'.$icons[$key].'&nbsp;', $string);
          >                     $newstring = str_replace('" >', '">'.$icons[$key].'&nbsp;', $newstring);
          >                     if ($newstring == $string) {  // No link found, so insert before the string
          >                         $newstring = $icons[$key].'&nbsp;'.$string;
          >                     }
          >                     $string = $newstring;
          
          Show
          Martin Dougiamas added a comment - A (faster) patch to fix the blocks with icons ... it works for me but not sure about regressions (themes etc) so not sure about 1.9.6 ... Index: weblib.php =================================================================== RCS file: /cvsroot/moodle/moodle/lib/weblib.php,v retrieving revision 1.970.2.144 diff -r1.970.2.144 weblib.php 6434,6435c6434,6440 < if ($icons) { < echo '<div class="icon column c0">'. $icons[$key] .'</div>'; --- > if ($icons) { // Hacky way to insert the icon into the link. See MDL-6820 > $newstring = str_replace('">', '">'.$icons[$key].'&nbsp;', $string); > $newstring = str_replace('" >', '">'.$icons[$key].'&nbsp;', $newstring); > if ($newstring == $string) { // No link found, so insert before the string > $newstring = $icons[$key].'&nbsp;'.$string; > } > $string = $newstring;
          Hide
          Dan Poltawski added a comment -

          This just screams out to me as a Moodle 2.0 change where we fix it at the source rather than search and replace bodges

          Show
          Dan Poltawski added a comment - This just screams out to me as a Moodle 2.0 change where we fix it at the source rather than search and replace bodges
          Hide
          Anthony Borrow added a comment -

          Based on Martin's closer review, my +1 for 2.0

          Show
          Anthony Borrow added a comment - Based on Martin's closer review, my +1 for 2.0
          Hide
          Rossiani Wijaya added a comment -

          Martin,

          I made slight changes to your patch.

          the new patch will insert the icon into the first link only (please refer to MDL-20725 for description and screenshot).

          Show
          Rossiani Wijaya added a comment - Martin, I made slight changes to your patch. the new patch will insert the icon into the first link only (please refer to MDL-20725 for description and screenshot).
          Hide
          Rossiani Wijaya added a comment -

          The patch has been checked-in.

          Show
          Rossiani Wijaya added a comment - The patch has been checked-in.
          Hide
          Andrew Davis added a comment -

          The icons for course activities in the centre are clickable. As are the icons in the Administration, Course Categories and Participants block. Oddly the icons in the Upcoming Events block are not clickable. Is that a problem?

          Show
          Andrew Davis added a comment - The icons for course activities in the centre are clickable. As are the icons in the Administration, Course Categories and Participants block. Oddly the icons in the Upcoming Events block are not clickable. Is that a problem?
          Hide
          Sam Hemelryk added a comment -

          This has been discussed between Rossi, Martin, and myself and the recent changes have now been reverted due to the complications that they introduced.
          Sorry to all of those who were hanging out for this in 1.9 it will now be fixed (properly) in 2.0, Rossi is currently working on a patch for this.
          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - This has been discussed between Rossi, Martin, and myself and the recent changes have now been reverted due to the complications that they introduced. Sorry to all of those who were hanging out for this in 1.9 it will now be fixed (properly) in 2.0, Rossi is currently working on a patch for this. Cheers Sam
          Hide
          Rossiani Wijaya added a comment -

          Attached Patch for 2.0

          Please take a look and feel free to leave any comment.

          Thanks
          Rosie

          Show
          Rossiani Wijaya added a comment - Attached Patch for 2.0 Please take a look and feel free to leave any comment. Thanks Rosie
          Hide
          Rossiani Wijaya added a comment - - edited

          Commit changes to 2.0.

          Show
          Rossiani Wijaya added a comment - - edited Commit changes to 2.0.

            People

            • Votes:
              19 Vote for this issue
              Watchers:
              12 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: