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

Username may overlap userreport icon in the grader report

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.1.3, 2.2
    • Fix Version/s: 2.0.7, 2.1.4, 2.2.1
    • Component/s: Gradebook
    • Labels:
    • Testing Instructions:
      Hide

      1. Make sure you have a user with a very long name, and a course with several activities in the gradebook.

      2. Go to the gradebook, and make sure the option to display user picture is on.

      3. Verify that the user's name does not overlap the icon to go to the user's user report.

      Please check this all all major browsers, and with several themes.

      Show
      1. Make sure you have a user with a very long name, and a course with several activities in the gradebook. 2. Go to the gradebook, and make sure the option to display user picture is on. 3. Verify that the user's name does not overlap the icon to go to the user's user report. Please check this all all major browsers, and with several themes.
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull from Repository:
    • Pull Master Branch:

      Description

      1. Go to the grader report using Firefox (not a webkit browser).
      2. Make sure that the user profile images, and the links to the user report are visible.
      3. Make sure that one of the users at least has a long name, and then make your browser window narrow.

      Observe that it is possible for the name to overlap the user report icon.

        Gliffy Diagrams

          Attachments

            Activity

            Hide
            timhunt Tim Hunt added a comment -

            I have done two alternative fixes:

            https://github.com/timhunt/moodle/compare/master...MDL-30498a is a 'better' fix, in that it removes what seems to me to be an unnecessary div.

            https://github.com/timhunt/moodle/compare/master...MDL-30498b is a safer fix, in that it only changes the CSS. This is less likely to break people's themes.

            Please could you take a look, and let me know which change you think we should go for. Thanks.

            Show
            timhunt Tim Hunt added a comment - I have done two alternative fixes: https://github.com/timhunt/moodle/compare/master...MDL-30498a is a 'better' fix, in that it removes what seems to me to be an unnecessary div. https://github.com/timhunt/moodle/compare/master...MDL-30498b is a safer fix, in that it only changes the CSS. This is less likely to break people's themes. Please could you take a look, and let me know which change you think we should go for. Thanks.
            Hide
            timhunt Tim Hunt added a comment -

            Adding some other grade-y people as watchers.

            Show
            timhunt Tim Hunt added a comment - Adding some other grade-y people as watchers.
            Hide
            timhunt Tim Hunt added a comment -

            Quick screen-grab to show the problem

            Just to repeat, this problem happens in Firefox.

            There is no problem in Chrome/Safari.

            There is a different layout problem in IE (9) and either of my patches fixes that too.

            Show
            timhunt Tim Hunt added a comment - Quick screen-grab to show the problem Just to repeat, this problem happens in Firefox. There is no problem in Chrome/Safari. There is a different layout problem in IE (9) and either of my patches fixes that too.
            Hide
            timhunt Tim Hunt added a comment -

            Ping! Andrew, or someone. I would really appreciate some feedback on this.

            Show
            timhunt Tim Hunt added a comment - Ping! Andrew, or someone. I would really appreciate some feedback on this.
            Hide
            andyjdavis Andrew Davis added a comment -

            Sorry for the delay Tim. Having a look now

            Show
            andyjdavis Andrew Davis added a comment - Sorry for the delay Tim. Having a look now
            Hide
            andyjdavis Andrew Davis added a comment - - edited

            I'd like to go with option A (removing the apparently unnecessary div) if at all possible. It's always nice to strip some of the unnecessary bits out of our html and css. We're right out a leaf of the html tree (if that makes sense) so any theme breakages should be relatively minor.

            Make sure you add testing instructions. You can just say "see the description" but specify that this fix should be tested in FF, IE, Chrome and probably Safari as well even though the original bug didn't necessarily appear in those browsers.

            Show
            andyjdavis Andrew Davis added a comment - - edited I'd like to go with option A (removing the apparently unnecessary div) if at all possible. It's always nice to strip some of the unnecessary bits out of our html and css. We're right out a leaf of the html tree (if that makes sense) so any theme breakages should be relatively minor. Make sure you add testing instructions. You can just say "see the description" but specify that this fix should be tested in FF, IE, Chrome and probably Safari as well even though the original bug didn't necessarily appear in those browsers.
            Hide
            timhunt Tim Hunt added a comment -

            Thanks Andrew. Submitting for integration now.

            Show
            timhunt Tim Hunt added a comment - Thanks Andrew. Submitting for integration now.
            Hide
            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
            samhemelryk Sam Hemelryk added a comment -

            Thanks Tim - this has been integrated now

            Show
            samhemelryk Sam Hemelryk added a comment - Thanks Tim - this has been integrated now
            Hide
            rwijaya Rossiani Wijaya added a comment -

            This looks good.

            Test passed.

            Show
            rwijaya Rossiani Wijaya added a comment - This looks good. Test passed.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Whoever decided one week was worth 14 days had really one bad idea. Anyway, the nightmare is over, so thanks for your, once again, amazing contributions. Many, many thanks!

            Now... disconnect, relax and enjoy the next days, yay!

            Closing...ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Whoever decided one week was worth 14 days had really one bad idea. Anyway, the nightmare is over, so thanks for your, once again, amazing contributions. Many, many thanks! Now... disconnect, relax and enjoy the next days, yay! Closing...ciao

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  9/Jan/12