Moodle
  1. Moodle
  2. MDL-37074

YUI replaces element classes instead of concatenating

    Details

    • Testing Instructions:
      Hide

      (Requires developer tools ie firefox/chrome)

      1. Clear all caches
      2. Go to the assignment grading page
      3. Select a row from the grading table
      4. Right click on a cell in the row and "Inspect" it
      5. Look at the containing <tr> element
      6. Verify it has both "selectedrow" and "r0" or "r1" classes
      Show
      (Requires developer tools ie firefox/chrome) Clear all caches Go to the assignment grading page Select a row from the grading table Right click on a cell in the row and "Inspect" it Look at the containing <tr> element Verify it has both "selectedrow" and "r0" or "r1" classes
    • Affected Branches:
      MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-37074-master

      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

        Gliffy Diagrams

          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: