Details

    • Type: Sub-task
    • Status: Closed
    • Priority: 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

      Description

      Redesign Grader report icons in svg and png formats

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            barbararamiro 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
            barbararamiro 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
            fred Frédéric Massart added a comment -

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

            Show
            fred Frédéric Massart added a comment - Assigning this to myself as I will commit the icons for Barbara.
            Hide
            barbararamiro 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
            barbararamiro 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_frenz 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_frenz 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_frenz Ankit Agarwal added a comment -

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

            Show
            ankit_frenz Ankit Agarwal added a comment - Gradebook is missing svg icons as seen here http://www.uploadscreenshot.com/image/1649409/2325744
            Hide
            fred 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
            fred 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
            markn 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
            markn 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
            fred 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
            fred 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
            markn 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
            markn 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
            poltawski Dan Poltawski added a comment -

            Integrated, thanks.

            Show
            poltawski Dan Poltawski added a comment - Integrated, thanks.
            Hide
            dmonllao 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
            dmonllao 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
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Y E S !

            Closing as fixed, many thanks!

            Show
            stronk7 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:
                  Fix Release Date:
                  3/Dec/12