Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-34592

Grader Report rows do not align between usernames and their grades when editing is on or off. IE8, FF, Safari.

    Details

    • Testing Instructions:
      Hide
      1. Login as Admin and enable 'Allow theme change by URL in Theme settings page.
      2. Enable 'Static student' in Admin Grader settings
      3. Choose a course that has at least 5 students enrolled that will be listed in the Grader when viewed.
      4. Select 'Grades' from the Course Administration menu.
      5. TEST to see that the fixed column (Static students) columns rows and grader rows are not misaligned.
      6. With this page still open, add the following to the end of the course page URL in the address bar of your browser...

        &theme=base

        so that it looks similar to this...

        moodle/grade/report/grader/index.php?id=8&theme=base

        and again check that the columns rows and grader rows are not misaligned.

      7. Repeat 6 & 7 and TEST all CORE themes except splash and mymobile by changing the URL.
      Show
      Login as Admin and enable 'Allow theme change by URL in Theme settings page. Enable 'Static student' in Admin Grader settings Choose a course that has at least 5 students enrolled that will be listed in the Grader when viewed. Select 'Grades' from the Course Administration menu. TEST to see that the fixed column (Static students) columns rows and grader rows are not misaligned. With this page still open, add the following to the end of the course page URL in the address bar of your browser... &theme=base so that it looks similar to this... moodle/grade/report/grader/index.php?id=8&theme=base and again check that the columns rows and grader rows are not misaligned. Repeat 6 & 7 and TEST all CORE themes except splash and mymobile by changing the URL.
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull Master Branch:
      MDL-34592_master

      Description

      Using Moodle core themes, when accessing grader report the student names and their grades are offset by half a cell and do not align correctly.
      Tested with Magazine, Anomaly, Serenitiy, Boxxie.

        Gliffy Diagrams

        1. FF_editing _off.jpg
          194 kB
        2. FF.jpg
          226 kB
        3. IE8.jpg
          202 kB

          Issue Links

            Activity

            Hide
            salvetore Michael de Raadt added a comment -

            I think this has more to do with themes than the underlying gradebook. A similar problem was reported relating to a non-standard theme. One of the comments on that issue was...

            I had this problem too. With theme aardvark_postit.

            I have solution:
            \grade\report\grader\lib.php
            insert - $fillercell->header = true;

            public function get_left_rows() {
            ...
            for ($i = 0; $i < $levels; $i++)

            Unknown macro: { $fillercell = new html_table_cell(); $fillercell->attributes['class'] = 'fixedcolumn cell topleft'; $fillercell->text = ' '; /* here */ $fillercell->header = true; $fillercell->colspan = $colspan; $row = new html_table_row(array($fillercell)); $rows[] = $row; }
            Show
            salvetore Michael de Raadt added a comment - I think this has more to do with themes than the underlying gradebook. A similar problem was reported relating to a non-standard theme. One of the comments on that issue was... I had this problem too. With theme aardvark_postit. I have solution: \grade\report\grader\lib.php insert - $fillercell->header = true; public function get_left_rows() { ... for ($i = 0; $i < $levels; $i++) Unknown macro: { $fillercell = new html_table_cell(); $fillercell->attributes['class'] = 'fixedcolumn cell topleft'; $fillercell->text = ' '; /* here */ $fillercell->header = true; $fillercell->colspan = $colspan; $row = new html_table_row(array($fillercell)); $rows[] = $row; }
            Hide
            snydert Tamara Snyder added a comment -

            I believe this affects version 2.2 as well. We are using Moodle 2.2, our theme is based on Magazine, we also have this problem in IE, not FF.

            Show
            snydert Tamara Snyder added a comment - I believe this affects version 2.2 as well. We are using Moodle 2.2, our theme is based on Magazine, we also have this problem in IE, not FF.
            Hide
            bethcrook Beth Crook added a comment - - edited

            I wish we could find a fix for this. This is the make or break deal of whether or not our faculty will use Moodle.

            We tried adding the code in lib.php, but that didn't correct it. We are using Chrome and Safari on macs.

            Show
            bethcrook Beth Crook added a comment - - edited I wish we could find a fix for this. This is the make or break deal of whether or not our faculty will use Moodle. We tried adding the code in lib.php, but that didn't correct it. We are using Chrome and Safari on macs.
            Hide
            cck208 Carly Klimash added a comment -

            We are also using a theme based off of magazine. We modified core.css for the theme to remove padding for the generaltable header.
            Keep in mind, removing padding for the generaltable header will impact padding on all tables using it in moodle, not just the gradebook. Our testing indicated limited significance.

            The following was modified in file - moodleroot/theme/(themename)/style/core.css

            /*td.maincalendar table.calendartable th, table.rolecap .header,.generaltable .header, (remove this from here) .forumheaderlist .header,.files .header,.editcourse .header,.logtable .header,#attempts .header,table#categoryquestions th {*/
            td.maincalendar table.calendartable th, table.rolecap .header,.forumheaderlist .header,.files .header,.editcourse .header,.logtable .header,#attempts .header,table#categoryquestions th {
            font-size: 11px;
            font-weight: 200;
            text-decoration: none;
            color: #fff !important;
            border-top: 1px solid #ccc !important;
            padding: 5px;
            }

            /*(add separately here WITHOUT padding) added this section separately without padding to deal with gradebook alignment issues*/
            .generaltable .header {
            font-size: 11px;
            font-weight: 200;
            text-decoration: none;
            color: #fff !important;
            border-top: 1px solid #ccc !important;
            }

            Show
            cck208 Carly Klimash added a comment - We are also using a theme based off of magazine. We modified core.css for the theme to remove padding for the generaltable header. Keep in mind, removing padding for the generaltable header will impact padding on all tables using it in moodle, not just the gradebook. Our testing indicated limited significance. The following was modified in file - moodleroot/theme/(themename)/style/core.css /* td.maincalendar table.calendartable th, table.rolecap .header,.generaltable .header, (remove this from here) .forumheaderlist .header,.files .header,.editcourse .header,.logtable .header,#attempts .header,table#categoryquestions th { */ td.maincalendar table.calendartable th, table.rolecap .header,.forumheaderlist .header,.files .header,.editcourse .header,.logtable .header,#attempts .header,table#categoryquestions th { font-size: 11px; font-weight: 200; text-decoration: none; color: #fff !important; border-top: 1px solid #ccc !important; padding: 5px; } /* (add separately here WITHOUT padding) added this section separately without padding to deal with gradebook alignment issues */ .generaltable .header { font-size: 11px; font-weight: 200; text-decoration: none; color: #fff !important; border-top: 1px solid #ccc !important; }
            Hide
            lazydaisy Mary Evans added a comment -

            I'm attempting to fix this now. I was initially in the dark about the 'Static Students' setting in Grader settings, so did not see the mis-alignment until in the last fortnight or so.

            I can confirm it is the 5px padding that is causing this. The fact this it's also a problem in Aardvark Postit-IT is because it was based on a theme that uses the same block of code.

            Show
            lazydaisy Mary Evans added a comment - I'm attempting to fix this now. I was initially in the dark about the 'Static Students' setting in Grader settings, so did not see the mis-alignment until in the last fortnight or so. I can confirm it is the 5px padding that is causing this. The fact this it's also a problem in Aardvark Postit-IT is because it was based on a theme that uses the same block of code.
            Hide
            lazydaisy Mary Evans added a comment -

            On closer inspection it appears to be a problem in ALL themes, it happens in Base theme and Canvas. The problem is in the grader style.css.

            Show
            lazydaisy Mary Evans added a comment - On closer inspection it appears to be a problem in ALL themes, it happens in Base theme and Canvas. The problem is in the grader style.css.
            Hide
            lazydaisy Mary Evans added a comment - - edited

            Note to Integrator:

            Please observe master and MOODLE_24_STABLE branches has the same CSS for the following line in grade/report/grader/style.css...

            .path-grade-report-grader th {padding:2px 10px;}

            which differers from MOODLE_23_STABLE and MOODLE_22_STABLE which gives this...

            .path-grade-report-grader th {padding:2px 10px 0;}

            These give two very different results:

            padding: 2px 10px; 

            gives the table cell a 2px padding top and bottom, and 10px padding on both sides (left & right), whereas...

            padding: 2px 10px 0; 

            gives the table cell a 2px padding to top, 10px padding on bother sides (left & right), and NO padding at the bottom of the cell.

            So in Moodle 2.4 ALL themes displayed a slight row misalignment when Static student (fixed columns) was enabled. This was not true in Moodle 2.3 & Moodle 2.2, although Magazine theme did display badly in each branch to-date.

            With the current commits I have made the grader CSS the same in all stable branches and master, as you will see in the commits.

            And fixed Magazine in all branches.

            Thanks and Good night all.

            Mary

            Show
            lazydaisy Mary Evans added a comment - - edited Note to Integrator: Please observe master and MOODLE_24_STABLE branches has the same CSS for the following line in grade/report/grader/style.css... .path-grade-report-grader th {padding:2px 10px;} which differers from MOODLE_23_STABLE and MOODLE_22_STABLE which gives this... .path-grade-report-grader th {padding:2px 10px 0;} These give two very different results: padding: 2px 10px; gives the table cell a 2px padding top and bottom, and 10px padding on both sides (left & right), whereas... padding: 2px 10px 0; gives the table cell a 2px padding to top, 10px padding on bother sides (left & right), and NO padding at the bottom of the cell. So in Moodle 2.4 ALL themes displayed a slight row misalignment when Static student (fixed columns) was enabled. This was not true in Moodle 2.3 & Moodle 2.2, although Magazine theme did display badly in each branch to-date. With the current commits I have made the grader CSS the same in all stable branches and master, as you will see in the commits. And fixed Magazine in all branches. Thanks and Good night all. Mary
            Hide
            lazydaisy Mary Evans added a comment -

            Postcript: I don't have Safari or IE8 so could not test.

            Show
            lazydaisy Mary Evans added a comment - Postcript: I don't have Safari or IE8 so could not test.
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Thanks Mary, has been integrated now!

            Show
            samhemelryk Sam Hemelryk added a comment - Thanks Mary, has been integrated now!
            Hide
            damyon Damyon Wiese added a comment -

            Testing in firefox:

            Passed:
            afterburner, anomaly, arialist, base, binarius, boxxie, brick, canvas, format_white, formfactor, fusion, leatherbound, nimble, nonzero, overlay, serenity, sky_high, standard, standardold

            Failed:
            magazine (1 or 2 pixels out)
            mymobile (10 pixels out)
            splash (10 pixels out)

            Testing in IE8:
            Passed:
            afterburner, anomaly, arialist, brick, nimble, nonzero, serenity, sky_high,

            Failed:
            base (1 pixel out)
            binarius (1 pixel out)
            boxxie (1 pixel out)
            canvas (1 pixel out)
            formal_white (1 pixel out)
            formfactor (1 pixel out)
            fusion (1 pixel out)
            leatherbound (1 pixel out)
            magazine (2 pixels out)
            mymobile (10 pixels out)
            overlay (1 pixel out)
            splash (10-20 pixels out)
            standard (1 pixel out)
            standardold (1 pixel out)

            The issues marked as 1 pixel out were subtle (the line separating the rows did not look continuous) and are a minor issue, however the misalignment is more severe in splash, mymoble and magazine.

            Show
            damyon Damyon Wiese added a comment - Testing in firefox: Passed: afterburner, anomaly, arialist, base, binarius, boxxie, brick, canvas, format_white, formfactor, fusion, leatherbound, nimble, nonzero, overlay, serenity, sky_high, standard, standardold Failed: magazine (1 or 2 pixels out) mymobile (10 pixels out) splash (10 pixels out) Testing in IE8: Passed: afterburner, anomaly, arialist, brick, nimble, nonzero, serenity, sky_high, Failed: base (1 pixel out) binarius (1 pixel out) boxxie (1 pixel out) canvas (1 pixel out) formal_white (1 pixel out) formfactor (1 pixel out) fusion (1 pixel out) leatherbound (1 pixel out) magazine (2 pixels out) mymobile (10 pixels out) overlay (1 pixel out) splash (10-20 pixels out) standard (1 pixel out) standardold (1 pixel out) The issues marked as 1 pixel out were subtle (the line separating the rows did not look continuous) and are a minor issue, however the misalignment is more severe in splash, mymoble and magazine.
            Hide
            damyon Damyon Wiese added a comment -

            I don't have safari handy so I haven't completed the test since there are already failures.

            Show
            damyon Damyon Wiese added a comment - I don't have safari handy so I haven't completed the test since there are already failures.
            Hide
            lazydaisy Mary Evans added a comment -

            Damyon,

            Splash is not picking up Base theme CSS for the grader as it is still showing a 20px padding to the left side, which is actually in the grader style.css. I didn't notice it when I was fixing the issue, as it is fixed in Base theme, but for some reason Splash is not picking it up. Also after many changes to Splash between 2.2 and 2.3 or was it between 2.3 and 2.4 (I forget), anyway there seems to be some duplication of CSS, and the .generaltable .header code is repeated in Splash core.css which is, I suppose, partly my fault as I did the original changes to that theme!

            Can this not be passed, as the problems with My Mobile and Splash are very noticeable, even without these new changes.

            We could set up new trackers as Found while testing) issues so that I can fix them independently.

            Thanks
            Mary

            Show
            lazydaisy Mary Evans added a comment - Damyon, Splash is not picking up Base theme CSS for the grader as it is still showing a 20px padding to the left side, which is actually in the grader style.css. I didn't notice it when I was fixing the issue, as it is fixed in Base theme, but for some reason Splash is not picking it up. Also after many changes to Splash between 2.2 and 2.3 or was it between 2.3 and 2.4 (I forget), anyway there seems to be some duplication of CSS, and the .generaltable .header code is repeated in Splash core.css which is, I suppose, partly my fault as I did the original changes to that theme! Can this not be passed, as the problems with My Mobile and Splash are very noticeable, even without these new changes. We could set up new trackers as Found while testing) issues so that I can fix them independently. Thanks Mary
            Hide
            damyon Damyon Wiese added a comment -

            Updating the testing instructions to exclude splash and mymobile - I'll create a separate issue for them in order to get this passed.

            Show
            damyon Damyon Wiese added a comment - Updating the testing instructions to exclude splash and mymobile - I'll create a separate issue for them in order to get this passed.
            Hide
            damyon Damyon Wiese added a comment -

            OK - Mary - I have tested with safari and get the same results.

            If we reset this test I'm happy to pass it given the updated test instructions.

            Show
            damyon Damyon Wiese added a comment - OK - Mary - I have tested with safari and get the same results. If we reset this test I'm happy to pass it given the updated test instructions.
            Hide
            lazydaisy Mary Evans added a comment -

            That would be great Damyon, thanks

            I'll set up new tracker issues to deal with Splash and MyMobile.

            Show
            lazydaisy Mary Evans added a comment - That would be great Damyon, thanks I'll set up new tracker issues to deal with Splash and MyMobile.
            Hide
            lazydaisy Mary Evans added a comment - - edited

            @Dan or Damyon
            I have just found where the extra padding came from.

            https://github.com/moodle/moodle/commit/0cddd851517d2ddb4f73530fce707e4a7b5c262d

            This was done a month ago by Frédéric Massart.
            This relates to the Note to Integrator which I wrote here on the 10th December. See above.

            Show
            lazydaisy Mary Evans added a comment - - edited @Dan or Damyon I have just found where the extra padding came from. https://github.com/moodle/moodle/commit/0cddd851517d2ddb4f73530fce707e4a7b5c262d This was done a month ago by Frédéric Massart. This relates to the Note to Integrator which I wrote here on the 10th December. See above.
            Hide
            lazydaisy Mary Evans added a comment -

            Just added Frédéric Massart as a watcher.

            Show
            lazydaisy Mary Evans added a comment - Just added Frédéric Massart as a watcher.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Many thanks for your effort, the whole Moodle Community will be enjoying your great solutions starting now!

            Closing, ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Many thanks for your effort, the whole Moodle Community will be enjoying your great solutions starting now! Closing, ciao
            Hide
            msohail99 Sohail Aslam added a comment -

            Not sure where to ask this question.
            I am new to moodle tracker.
            This issue details says, affects version 2.3.1 , fix versions 2.2.7, 2.3.4, 2.4.1.

            I am using moodle 2.3.1. What exactly I need to do to implement this fix to my site (which files and what needs to be updated).

            Show
            msohail99 Sohail Aslam added a comment - Not sure where to ask this question. I am new to moodle tracker. This issue details says, affects version 2.3.1 , fix versions 2.2.7, 2.3.4, 2.4.1. I am using moodle 2.3.1. What exactly I need to do to implement this fix to my site (which files and what needs to be updated).
            Hide
            fred Frédéric Massart added a comment -

            Hi Sohail,

            Affected versions are the version we are sure are being affected by the issue, often this field is not stating the complete list of affected versions. The fix version is the version in which the patch has been introduced. I would usually not recommend you to try to implement the fix manually especially one from a version higher than the one you're using, you could very quickly encounter random issues. But in this case, it appears that those are only a few CSS changes:

            https://github.com/lazydaisy/moodle/commit/564b28731c37f1002f74ad11b9ec95c32c600530

            Once you have applied those changes, purge your cache and it should be it. If it does not change anything, then you should probably update your Moodle to the latest 2.3 or even migrate to 2.4 if you want to benefit the changes of the latest major release.

            I hope this helps,

            Fred

            Show
            fred Frédéric Massart added a comment - Hi Sohail, Affected versions are the version we are sure are being affected by the issue, often this field is not stating the complete list of affected versions. The fix version is the version in which the patch has been introduced. I would usually not recommend you to try to implement the fix manually especially one from a version higher than the one you're using, you could very quickly encounter random issues. But in this case, it appears that those are only a few CSS changes: https://github.com/lazydaisy/moodle/commit/564b28731c37f1002f74ad11b9ec95c32c600530 Once you have applied those changes, purge your cache and it should be it. If it does not change anything, then you should probably update your Moodle to the latest 2.3 or even migrate to 2.4 if you want to benefit the changes of the latest major release. I hope this helps, Fred
            Hide
            msohail99 Sohail Aslam added a comment -

            Thanks Frederic for your quick response and explaining this to me.

            I will give it a go.

            Show
            msohail99 Sohail Aslam added a comment - Thanks Frederic for your quick response and explaining this to me. I will give it a go.
            Hide
            lazydaisy Mary Evans added a comment -

            I've just noticed a typo in the CSS for MOODLE_23_STABLE diff https://github.com/lazydaisy/moodle/commit/564b28731c37f1002f74ad11b9ec95c32c600530

            The CSS is missing 'a' off the end of the second .generaltable .header it should read...

            .generaltable .header,
            .generaltable .header a {
                padding: 0
            }

            Show
            lazydaisy Mary Evans added a comment - I've just noticed a typo in the CSS for MOODLE_23_STABLE diff https://github.com/lazydaisy/moodle/commit/564b28731c37f1002f74ad11b9ec95c32c600530 The CSS is missing 'a' off the end of the second .generaltable .header it should read... .generaltable .header, .generaltable .header a { padding: 0 }
            Hide
            gideonwilliams Gideon Williams added a comment -

            Also affects Moodle 2.5 But fix works too :O)

            Show
            gideonwilliams Gideon Williams added a comment - Also affects Moodle 2.5 But fix works too :O)
            Hide
            adamann2 Ann Adamcik added a comment -

            This seems to have reappeared in 2.5.1, with the Clean / Bootstrap themes.

            We have the static student column enabled, and the report rows are significantly misaligned when editing is turned on. The issue exists with Firefox and Chrome (I haven't tested other browsers).

            Let me know if I should open a new issue.

            Show
            adamann2 Ann Adamcik added a comment - This seems to have reappeared in 2.5.1, with the Clean / Bootstrap themes. We have the static student column enabled, and the report rows are significantly misaligned when editing is turned on. The issue exists with Firefox and Chrome (I haven't tested other browsers). Let me know if I should open a new issue.
            Hide
            wartlydi Lydia Warth added a comment -

            showing less students on the page (something around 5 per page) seems to be a good work around. but it isn't a great solution for faculty who are entering a lot of grades.

            Show
            wartlydi Lydia Warth added a comment - showing less students on the page (something around 5 per page) seems to be a good work around. but it isn't a great solution for faculty who are entering a lot of grades.
            Hide
            lazydaisy Mary Evans added a comment -

            There is no point commenting in this Tracker issue as it is closed. If there is a problem in Clean theme then you need to create a new tracker to report it.
            Thank you

            Show
            lazydaisy Mary Evans added a comment - There is no point commenting in this Tracker issue as it is closed. If there is a problem in Clean theme then you need to create a new tracker to report it. Thank you
            Hide
            beachtch Lisa Beach added a comment -

            We are using the Clean theme and have instructors complaining about this. Is there a new Tracker issue? If not, do you suggest we go back to using a different theme?

            Show
            beachtch Lisa Beach added a comment - We are using the Clean theme and have instructors complaining about this. Is there a new Tracker issue? If not, do you suggest we go back to using a different theme?
            Hide
            fred Frédéric Massart added a comment -

            Hi Lisa,

            this issue is closed, and should have been fixed. What version of Moodle are you using? If you are using a recent version can I ask you to raise a new issue, including accurate replication steps, and then we will get onto it to fix that bug you are experiencing.

            Many thanks.
            Fred

            Show
            fred Frédéric Massart added a comment - Hi Lisa, this issue is closed, and should have been fixed. What version of Moodle are you using? If you are using a recent version can I ask you to raise a new issue, including accurate replication steps, and then we will get onto it to fix that bug you are experiencing. Many thanks. Fred
            Hide
            beachtch Lisa Beach added a comment -

            Thanks, Fred. I've never created a new "issue" but I'll give it a shot.

            Lisa

            Show
            beachtch Lisa Beach added a comment - Thanks, Fred. I've never created a new "issue" but I'll give it a shot. Lisa

              People

              • Votes:
                4 Vote for this issue
                Watchers:
                11 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  14/Jan/13