Moodle
  1. Moodle
  2. MDL-37074

YUI replaces element classes instead of concatenating

    Details

    • Rank:
      46628

      Description

      The gradebook table (.gradingtable) table rows contain the classes .r0 and .r1, but when YUI loads it replaces those classes with .rowunselected and replaces that with .rowselected if the checkbox is checked.

      I think that it should be concatenated to be ".r0 .rowunselected" and ".r1 .rowunselected" so that the table can continue to leverage the benefits of striping

        Activity

        Hide
        Andrew Nicols added a comment -

        Hi Daniel,

        Could you provide some instructions on how this - which gradebook table do you mean? Do you have any specific settings enabled - e.g. AJAX grading?

        Looking at /grade/report/grader/index.php I see a table with classes gradestable rather than gradingtable.

        Cheers,

        Andrew

        Show
        Andrew Nicols added a comment - Hi Daniel, Could you provide some instructions on how this - which gradebook table do you mean? Do you have any specific settings enabled - e.g. AJAX grading? Looking at /grade/report/grader/index.php I see a table with classes gradestable rather than gradingtable. Cheers, Andrew
        Hide
        Andrew Nicols added a comment -

        Ah - just worked out you mean the assignment module /mod/assign/view.php?id=11&action=grading

        Show
        Andrew Nicols added a comment - Ah - just worked out you mean the assignment module /mod/assign/view.php?id=11&action=grading
        Hide
        Andrew Nicols added a comment -

        Adding Damyon as a watcher as he is the master of the assignment module.

        Show
        Andrew Nicols added a comment - Adding Damyon as a watcher as he is the master of the assignment module.
        Hide
        Andrew Nicols added a comment -

        Looking at this further, it's not a case of YUI replacing the r0 and r1 classes, but that they're also not present in the first place.

        The JS would indeed replace them too (it calls rowelement.setAttribute('class', 'selectedrow'), but this isn't a YUI issue IMHO.

        Transferring to Damyon.

        Show
        Andrew Nicols added a comment - Looking at this further, it's not a case of YUI replacing the r0 and r1 classes, but that they're also not present in the first place. The JS would indeed replace them too (it calls rowelement.setAttribute('class', 'selectedrow'), but this isn't a YUI issue IMHO. Transferring to Damyon.
        Hide
        David Scotson added a comment -

        Just a note that .r0 and .r1 classes aren't needed if you use nth-child(even) or nth-child(odd) in your CSS. Though this will of course cut off IE8 users:

        http://caniuse.com/css-sel3

        As well as smaller downloads from less unecessary code I've found the PHP to generate Moodle code gets surprisingly simpler when you stop having to add these classes. Add to that the benefit that anything can be striped in this way and at some point supporting IE8 with this feature no longer makes sense.

        Show
        David Scotson added a comment - Just a note that .r0 and .r1 classes aren't needed if you use nth-child(even) or nth-child(odd) in your CSS. Though this will of course cut off IE8 users: http://caniuse.com/css-sel3 As well as smaller downloads from less unecessary code I've found the PHP to generate Moodle code gets surprisingly simpler when you stop having to add these classes. Add to that the benefit that anything can be striped in this way and at some point supporting IE8 with this feature no longer makes sense.
        Hide
        Daniel Wahl added a comment -

        Andrew,

        Here you can see that it clearly does have the .r0 and .r1 classes that are stripped and replaced

        Show
        Daniel Wahl added a comment - Andrew, Here you can see that it clearly does have the .r0 and .r1 classes that are stripped and replaced
        Hide
        Daniel Wahl added a comment -

        @David,

        I agree that you can use :nth-child(odd) - which is what I have done in the interim. The issue I think is that the classes are already there, and styled in the themes, so now adding another rule more or less negates the savings of not printing the class name.

        The biggest issue however, as a theme designer, is that for the time being Moodle Core does support IE8, and therefore, so do I.

        Show
        Daniel Wahl added a comment - @David, I agree that you can use :nth-child(odd) - which is what I have done in the interim. The issue I think is that the classes are already there, and styled in the themes, so now adding another rule more or less negates the savings of not printing the class name. The biggest issue however, as a theme designer, is that for the time being Moodle Core does support IE8, and therefore, so do I.
        Hide
        Damyon Wiese added a comment -

        Simple fix.

        Show
        Damyon Wiese added a comment - Simple fix.
        Hide
        Dan Poltawski added a comment -

        Integrated to master, 24 and 23.

        Thanks Damyon

        Show
        Dan Poltawski added a comment - Integrated to master, 24 and 23. Thanks Damyon
        Hide
        Frédéric Massart added a comment -

        Passing, thanks! (displayed selectedrow, not selected, confirmed it was OK with Damyon.)

        Show
        Frédéric Massart added a comment - Passing, thanks! (displayed selectedrow , not selected , confirmed it was OK with Damyon.)
        Hide
        Damyon Wiese added a comment -

        Congratulations this fix has been added to Moodle!

        You may want to dedicate this issue to someone special on this Valentines day.

        Thanks!

        Show
        Damyon Wiese added a comment - Congratulations this fix has been added to Moodle! You may want to dedicate this issue to someone special on this Valentines day. Thanks!

          People

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

            Dates

            • Created:
              Updated:
              Resolved: