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

When enrolling the user list in the pop-up window randomly colours the background of user rows

          Details

          • Testing Instructions:
            Hide
            1. Log in a Admin
            2. Go to a course manual enrolment page (Settings -> Course administration -> Users -> Enrolled users)
            3. Click enrol users
            4. In popup, inspect user rows (by firebug, drangonfly etc) and make sure they have alternate odd and even class.
            Show
            Log in a Admin Go to a course manual enrolment page (Settings -> Course administration -> Users -> Enrolled users) Click enrol users In popup, inspect user rows (by firebug, drangonfly etc) and make sure they have alternate odd and even class.
          • Affected Branches:
            MOODLE_22_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE
          • Fixed Branches:
            MOODLE_22_STABLE, MOODLE_23_STABLE
          • Pull from Repository:
          • Pull Master Branch:
            wip-mdl-34537
          • Pull Master Diff URL:

            Description

            There's CSS in the base theme which is intended to color odd and even rows of the user list different colors by attaching classes called "odd" and "even". The bit of code that adds these classes has a bug, since it adds the class based on the "rel" number (which might be the user id?) rather than on the position in the list. From the perspective of someone using Moodle this is effectively random.

            I've used the following CSS instead:

            .user-enroller-panel .uep-search-results .users .user:nth-child(odd)

            This doesn't work in IE8 but works everywhere else and doesn't require you to add classes to each row.

              Gliffy Diagrams

                Attachments

                  Activity

                  Ascending order - Click to sort in descending order
                  Hide
                  Permalink
                  salvetore Michael de Raadt added a comment -

                  Thanks for spotting that and suggesting a fix. If you could provide more context to your fix, that would be useful.

                  Show
                  salvetore Michael de Raadt added a comment - Thanks for spotting that and suggesting a fix. If you could provide more context to your fix, that would be useful.
                  Hide
                  Permalink
                  bawjaws David Scotson added a comment -

                  Bascially it's fancy new CSS3 but this article covers it in-depth:

                  http://dev.opera.com/articles/view/zebra-striping-tables-with-css3/

                  It also includes a javascript workaround for IE8 that I'm probably going to use, but haven't got around to yet.

                  Show
                  bawjaws David Scotson added a comment - Bascially it's fancy new CSS3 but this article covers it in-depth: http://dev.opera.com/articles/view/zebra-striping-tables-with-css3/ It also includes a javascript workaround for IE8 that I'm probably going to use, but haven't got around to yet.
                  Hide
                  Permalink
                  fred Frédéric Massart added a comment -

                  Looks good guys. Submitting for integration!

                  Show
                  fred Frédéric Massart added a comment - Looks good guys. Submitting for integration!
                  Hide
                  Permalink
                  stronk7 Eloy Lafuente (stronk7) added a comment -

                  The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

                  TIA and ciao

                  Show
                  stronk7 Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
                  Hide
                  Permalink
                  nebgor Aparup Banerjee added a comment -

                  yes this does make better sense. thanks for the patch.
                  integrated into 22,23 and master for testing.

                  ps: i couldn't replicate but certainly makes sense code wise. added Mary here too just for her eyes

                  Show
                  nebgor Aparup Banerjee added a comment - yes this does make better sense. thanks for the patch. integrated into 22,23 and master for testing. ps: i couldn't replicate but certainly makes sense code wise. added Mary here too just for her eyes
                  Hide
                  Permalink
                  lazydaisy Mary Evans added a comment - - edited

                  This sounds to me that we are using CSS to fix a BUG when in actual fact the BUG has not been fixed. Surely this should be fixed at source, meaning CORE code, shouldn't it? If this patch is the best way we can fix it well so be it, but I am not happy about it.

                  Sorry, I should have looked at the diff code befor I spoke!
                  You fixed the BUG!

                  Cool...looks good!

                  Show
                  lazydaisy Mary Evans added a comment - - edited This sounds to me that we are using CSS to fix a BUG when in actual fact the BUG has not been fixed. Surely this should be fixed at source, meaning CORE code, shouldn't it? If this patch is the best way we can fix it well so be it, but I am not happy about it. Sorry, I should have looked at the diff code befor I spoke! You fixed the BUG! Cool...looks good!
                  Hide
                  Permalink
                  nebgor Aparup Banerjee added a comment -

                  hehe its always interesting to read spoken words :-D , thanks for looking Mary!

                  Show
                  nebgor Aparup Banerjee added a comment - hehe its always interesting to read spoken words :-D , thanks for looking Mary!
                  Hide
                  Permalink
                  abgreeve Adrian Greeve added a comment -

                  I replicated the bug and then tested on 2.2, 2.3 and Master.
                  Works as described.
                  Test passed.

                  Show
                  abgreeve Adrian Greeve added a comment - I replicated the bug and then tested on 2.2, 2.3 and Master. Works as described. Test passed.
                  Hide
                  Permalink
                  stronk7 Eloy Lafuente (stronk7) added a comment -

                  Many thanks for the hard work.

                  These changes have been spread upstream and are already available in the git and cvs repositories.

                  Ciao

                  Show
                  stronk7 Eloy Lafuente (stronk7) added a comment - Many thanks for the hard work. These changes have been spread upstream and are already available in the git and cvs repositories. Ciao

                    People

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

                      Dates

                      • Created:
                        Updated:
                        Resolved:
                        Fix Release Date:
                        10/Sep/12