Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3.3
    • Fix Version/s: 2.4
    • Component/s: Usability
    • Labels:
      None
    • Testing Instructions:
      Hide

      Test pre-requisites

      • A course with modules and users
      • Add outcomes to the gradebook
      • Add some grade items and categories
      • Enable/disable all the possible options in your grade report preferences

      Test steps

      1. Explore the gradebook widely
      2. Make sure
        • the icons are showing up and look nice
        • the fallback to PNG works
        • the padding/margin are good
        • and everything about the look and fell
      Show
      Test pre-requisites A course with modules and users Add outcomes to the gradebook Add some grade items and categories Enable/disable all the possible options in your grade report preferences Test steps Explore the gradebook widely Make sure the icons are showing up and look nice the fallback to PNG works the padding/margin are good and everything about the look and fell
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-36559-master-int
    • Rank:
      46035

      Description

      Redesign Grader report icons in svg and png formats

        Issue Links

          Activity

          Hide
          Barbara Ramiro added a comment -

          Fred, could you try to make all the activity icons display in 24x24 on all pages except on blocks then let me see how it looks. Thanks

          Show
          Barbara Ramiro added a comment - Fred, could you try to make all the activity icons display in 24x24 on all pages except on blocks then let me see how it looks. Thanks
          Hide
          Frédéric Massart added a comment -

          Assigning this to myself as I will commit the icons for Barbara.

          Show
          Frédéric Massart added a comment - Assigning this to myself as I will commit the icons for Barbara.
          Hide
          Barbara Ramiro added a comment -

          Fred, i know you dont usually attach screenshots for your issues but please do for all icon related issues to give the tester an idea visually in terms of icon image and layout. Thanks

          Show
          Barbara Ramiro added a comment - Fred, i know you dont usually attach screenshots for your issues but please do for all icon related issues to give the tester an idea visually in terms of icon image and layout. Thanks
          Hide
          Ankit Agarwal added a comment -

          Imo these help icons are too big http://www.uploadscreenshot.com/image/1649392/6665450 in grade/letter edit page. Please feel to ignore if you guys feel so.

          Show
          Ankit Agarwal added a comment - Imo these help icons are too big http://www.uploadscreenshot.com/image/1649392/6665450 in grade/letter edit page. Please feel to ignore if you guys feel so.
          Hide
          Ankit Agarwal added a comment -

          Gradebook is missing svg icons as seen here http://www.uploadscreenshot.com/image/1649409/2325744

          Show
          Ankit Agarwal added a comment - Gradebook is missing svg icons as seen here http://www.uploadscreenshot.com/image/1649409/2325744
          Hide
          Frédéric Massart added a comment - - edited

          Back in development to take care of:

          • folder icon for categories
          • t/locktime.gif
          • t/unlock_gray which should be renamed to t/locked
          • correspongind CSSing
          diff --git a/grade/lib.php b/grade/lib.php
          index 59bbc29..ff3c643 100644
          --- a/grade/lib.php
          +++ b/grade/lib.php
          @@ -1573,7 +1573,7 @@ class grade_structure {
                       $strparamobj->itemname = $element['object']->grade_item->itemname;
                       $strnonunlockable = get_string('nonunlockableverbose', 'grades', $strparamobj);
           
          -            $action = $OUTPUT->pix_icon('t/unlock_gray', $strnonunlockable);
          +            $action = $OUTPUT->pix_icon('t/locked', $strnonunlockable);
           
                   } else if ($element['object']->is_locked()) {
                       $type = 'unlock';
          diff --git a/grade/report/grader/styles.css b/grade/report/grader/styles.css
          index 829dc5c..e1bd564 100644
          --- a/grade/report/grader/styles.css
          +++ b/grade/report/grader/styles.css
          @@ -469,8 +469,8 @@ width:2000px;
           float:right;
           }
           
          -.path-grade-report-grader .action-icon img,
          -.path-grade-edit-tree .action-icon img { margin: 0 3px;}
          +.path-grade-report-grader img.smallicon,
          +.path-grade-edit-tree img.smallicon { margin: 0 3px;}
           
           .path-grade-report-grader .gradestable th.user,
           .path-grade-report-grader .gradestable th.range,
          
          Show
          Frédéric Massart added a comment - - edited Back in development to take care of: folder icon for categories t/locktime.gif t/unlock_gray which should be renamed to t/locked correspongind CSSing diff --git a/grade/lib.php b/grade/lib.php index 59bbc29..ff3c643 100644 --- a/grade/lib.php +++ b/grade/lib.php @@ -1573,7 +1573,7 @@ class grade_structure { $strparamobj->itemname = $element['object']->grade_item->itemname; $strnonunlockable = get_string('nonunlockableverbose', 'grades', $strparamobj); - $action = $OUTPUT->pix_icon('t/unlock_gray', $strnonunlockable); + $action = $OUTPUT->pix_icon('t/locked', $strnonunlockable); } else if ($element['object']->is_locked()) { $type = 'unlock'; diff --git a/grade/report/grader/styles.css b/grade/report/grader/styles.css index 829dc5c..e1bd564 100644 --- a/grade/report/grader/styles.css +++ b/grade/report/grader/styles.css @@ -469,8 +469,8 @@ width:2000px; float:right; } -.path-grade-report-grader .action-icon img, -.path-grade-edit-tree .action-icon img { margin: 0 3px;} +.path-grade-report-grader img.smallicon, +.path-grade-edit-tree img.smallicon { margin: 0 3px;} .path-grade-report-grader .gradestable th.user, .path-grade-report-grader .gradestable th.range,
          Hide
          Mark Nelson added a comment -

          Hi Fred, looks good.

          Few issues:

          1) I am not sure about replacing file_folder_icon() with $OUTPUT->pix_url, file_folder_icon contains a static cache array, so replacing this may reduce performance. However, I may be wrong about this as if it was such a performance saver surely it would be used for other icons rather than using $OUTPUT->pix_url.
          2) Good you are replacing print_arrow as it is deprecated.
          3) theme/base/style/grade.css, spacing between CSS attributes would be nice here - ie. ".gradetreebox th.actions

          {white-space:nowrap; width:105px;}

          4) Some of the '{' characters have spaces after them, some don't.
          5) Should you stick with px instead of em as this is used throughout Moodle?

          Show
          Mark Nelson added a comment - Hi Fred, looks good. Few issues: 1) I am not sure about replacing file_folder_icon() with $OUTPUT->pix_url, file_folder_icon contains a static cache array, so replacing this may reduce performance. However, I may be wrong about this as if it was such a performance saver surely it would be used for other icons rather than using $OUTPUT->pix_url. 2) Good you are replacing print_arrow as it is deprecated. 3) theme/base/style/grade.css, spacing between CSS attributes would be nice here - ie. ".gradetreebox th.actions {white-space:nowrap; width:105px;} 4) Some of the '{' characters have spaces after them, some don't. 5) Should you stick with px instead of em as this is used throughout Moodle?
          Hide
          Frédéric Massart added a comment -

          1) file_folder_icon() does not return the same icon as does pix_ico(). I changed it because we didn't want that icon from from f/ but the new one from i/.
          3, 4) I know, there is not guide lines.
          5) It's a designer choice. From Barbara's point of view we should have been using em everywhere, so where I can I used them.

          Cheers, pushing for integration!

          Show
          Frédéric Massart added a comment - 1) file_folder_icon() does not return the same icon as does pix_ico(). I changed it because we didn't want that icon from from f/ but the new one from i/. 3, 4) I know, there is not guide lines. 5) It's a designer choice. From Barbara's point of view we should have been using em everywhere, so where I can I used them. Cheers, pushing for integration!
          Hide
          Mark Nelson added a comment -

          I know the image is being pulled from another location but you are still taking away the caching aspect.

          Show
          Mark Nelson added a comment - I know the image is being pulled from another location but you are still taking away the caching aspect.
          Hide
          Dan Poltawski added a comment -

          Integrated, thanks.

          Show
          Dan Poltawski added a comment - Integrated, thanks.
          Hide
          David Monllaó added a comment -

          It passes, it looks VERY nice, I've enabled all the options and enabled/disabled locks and views. The gap between icons / text strings IMO seems to be appropiate

          Show
          David Monllaó added a comment - It passes, it looks VERY nice, I've enabled all the options and enabled/disabled locks and views. The gap between icons / text strings IMO seems to be appropiate
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Y E S !

          Closing as fixed, many thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Y E S ! Closing as fixed, many thanks!

            People

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

              Dates

              • Created:
                Updated:
                Resolved: