Moodle
  1. Moodle
  2. MDL-38657

Misalignment from center of names in generating User Report from Grades option

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.4.3
    • Fix Version/s: 2.3.7, 2.4.4
    • Component/s: Themes
    • Labels:
    • Rank:
      48704

      Description

      When one generates a User Report, and selects "All users" option from the right menu, a slight alignment occurs for the first name.

      Testing:

      1.Go to Course Administration > Grades
      2.Select User Report from the drop down menu
      3.Select All Users option from the right menu.

        Activity

        Hide
        Prateek Sachan added a comment - - edited

        I've updated my Github Moodle repository for master and MOODLE_24_STABLE branches. Here are the links:

        master branch: https://github.com/prateeksachan/moodle/compare/master...MDL-38657
        MOODLE_24_STABLE: https://github.com/prateeksachan/moodle/commit/608e7be39f8a4433bf4525d004a5b5c47bdf585f

        Show
        Prateek Sachan added a comment - - edited I've updated my Github Moodle repository for master and MOODLE_24_STABLE branches. Here are the links: master branch: https://github.com/prateeksachan/moodle/compare/master...MDL-38657 MOODLE_24_STABLE: https://github.com/prateeksachan/moodle/commit/608e7be39f8a4433bf4525d004a5b5c47bdf585f
        Hide
        Prateek Sachan added a comment -

        Does this look good?

        Show
        Prateek Sachan added a comment - Does this look good?
        Hide
        Andrew Nicols added a comment -

        Hi Mary,

        Hope you don't mind my assigning this to you. Thought you'd be the most appropriate person though to deal with it.

        I couldn't see a generic class in base defining clear:both which surprises me, but maybe I'm missing something?

        Andrew

        Show
        Andrew Nicols added a comment - Hi Mary, Hope you don't mind my assigning this to you. Thought you'd be the most appropriate person though to deal with it. I couldn't see a generic class in base defining clear:both which surprises me, but maybe I'm missing something? Andrew
        Hide
        Andrew Nicols added a comment -

        At a first glance though, this is using inline style on the element. We should look at stripping out the existing inline styles and putting them into the base theme, and using a style defining clear: both or something I guess.

        Will leave it for you to look at Mary.

        Andrew

        Show
        Andrew Nicols added a comment - At a first glance though, this is using inline style on the element. We should look at stripping out the existing inline styles and putting them into the base theme, and using a style defining clear: both or something I guess . Will leave it for you to look at Mary. Andrew
        Hide
        Mary Evans added a comment -

        Thank Prateek, but it does not work like that.

        The fix is pretty simple, it's just a matter of adding the following CSS to base theme...

        .path-grade-report-user h2.main {clear: both;}

        I'm not sure about the p tag inline style, I think that needs to be addressed in another tracker separately. I think it is an old fix from Moodle 1.9 and with the advent of bootstrap, may mean it gets removed.

        Show
        Mary Evans added a comment - Thank Prateek, but it does not work like that. The fix is pretty simple, it's just a matter of adding the following CSS to base theme... .path-grade-report-user h2.main {clear: both;} I'm not sure about the p tag inline style, I think that needs to be addressed in another tracker separately. I think it is an old fix from Moodle 1.9 and with the advent of bootstrap, may mean it gets removed.
        Hide
        Mary Evans added a comment -

        David Scotson I have just added you as a watcher here as I think the p tag in this report grader renderer could be removed or restyled in future to match "bootstrap" report layout syntax. What do you think?

        Mary

        Show
        Mary Evans added a comment - David Scotson I have just added you as a watcher here as I think the p tag in this report grader renderer could be removed or restyled in future to match "bootstrap" report layout syntax. What do you think? Mary
        Hide
        Prateek Sachan added a comment -

        Okay. No problem. Happy to help. I thought the CSS file was missing this particular style. And there was this p tag inline CSS. So I thought of adding it to it.

        Show
        Prateek Sachan added a comment - Okay. No problem. Happy to help. I thought the CSS file was missing this particular style. And there was this p tag inline CSS. So I thought of adding it to it.
        Hide
        Mary Evans added a comment - - edited

        The p tag is a throwback from Moodle 1.9 where tables were the order of the day. Plus the inline styles is definitely a sign it came from that era.

        Show
        Mary Evans added a comment - - edited The p tag is a throwback from Moodle 1.9 where tables were the order of the day. Plus the inline styles is definitely a sign it came from that era.
        Hide
        Dan Poltawski added a comment -

        Integrated to 23 24 and master, thanks Mary

        Show
        Dan Poltawski added a comment - Integrated to 23 24 and master, thanks Mary
        Hide
        Rajesh Taneja added a comment -

        Thanks for fixing this Mary,

        User names are centred in most of the themes except Splash and after burner theme.

        I am failing this because it removes float on "Select all or one user" selector, which adds a lot of gap in different themes and doesn't look reasonable.

        Adding few screen-shots. I will be happy to pass it, if you think this is acceptable.

        Show
        Rajesh Taneja added a comment - Thanks for fixing this Mary, User names are centred in most of the themes except Splash and after burner theme. I am failing this because it removes float on "Select all or one user" selector, which adds a lot of gap in different themes and doesn't look reasonable. Adding few screen-shots. I will be happy to pass it, if you think this is acceptable.
        Hide
        David Scotson added a comment -

        I personally prefer the "after" screenshot (for the theme in the screenshot, Brick, and Standard at least).

        Looking at it live in the browser, in the Brick theme that space is mostly created by large padding on top of the h2 and large margins set on .singleselect's so that theme appears to have made a choice to be more spaced out than Standard, a design decision you can see in the banner at the top as well. So I think lots of whitespace might have been the intention, even though it collapsed in this particular case due to the use of float, which often has upredictable effects like this.

        (And on a minor technical note, I don't think the change is removing the float, just "clearing" it, so that the following content starts below the select input, rather than alongside it.)

        Show
        David Scotson added a comment - I personally prefer the "after" screenshot (for the theme in the screenshot, Brick, and Standard at least). Looking at it live in the browser, in the Brick theme that space is mostly created by large padding on top of the h2 and large margins set on .singleselect's so that theme appears to have made a choice to be more spaced out than Standard, a design decision you can see in the banner at the top as well. So I think lots of whitespace might have been the intention, even though it collapsed in this particular case due to the use of float, which often has upredictable effects like this. (And on a minor technical note, I don't think the change is removing the float, just "clearing" it, so that the following content starts below the select input, rather than alongside it.)
        Hide
        David Scotson added a comment -

        I also noticed that the uncleared float causes strange behaviour when you first visit the page and it shows an "blank" table with no heading. If your screen is narrow enough then the table itself it shifted to the left, much like the heading is.

        Show
        David Scotson added a comment - I also noticed that the uncleared float causes strange behaviour when you first visit the page and it shows an "blank" table with no heading. If your screen is narrow enough then the table itself it shifted to the left, much like the heading is.
        Hide
        Mary Evans added a comment -

        Thanks David for looking at this. It brought to mind your comment about crazy themers fixing bugs in themes when the problem should be dealt with at source.

        Show
        Mary Evans added a comment - Thanks David for looking at this. It brought to mind your comment about crazy themers fixing bugs in themes when the problem should be dealt with at source.
        Hide
        Dan Poltawski added a comment -

        Sending back to testing because I think this can be passed?

        (For the record I love crazy themers! )

        Show
        Dan Poltawski added a comment - Sending back to testing because I think this can be passed? (For the record I love crazy themers! )
        Hide
        Rajesh Taneja added a comment -

        Thanks everyone,

        It would be nice to see if we can place select users drop-down in same line as choose plugin report drop-down.

        Passing it as suggested.

        Show
        Rajesh Taneja added a comment - Thanks everyone, It would be nice to see if we can place select users drop-down in same line as choose plugin report drop-down. Passing it as suggested.
        Hide
        Dan Poltawski added a comment -

        Did you remember to call thankDevelopers() for 'this_weeks_work'? Defaulting to PARAM_SHODDY thanking.

        line 1289 of \lib\changes.php: call to debugging()
        line 281 of \lib\are.php: call to moodleform->detectMissingThanks()
        line 202 of \lib\now.php: call to moodleform->_is_poor_form()
        line 73 of \course\upstream.php: call to moodleform->forgetingToThank()

        Show
        Dan Poltawski added a comment - Did you remember to call thankDevelopers() for 'this_weeks_work'? Defaulting to PARAM_SHODDY thanking. line 1289 of \lib\changes.php: call to debugging() line 281 of \lib\are.php: call to moodleform->detectMissingThanks() line 202 of \lib\now.php: call to moodleform->_is_poor_form() line 73 of \course\upstream.php: call to moodleform->forgetingToThank()

          People

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

            Dates

            • Created:
              Updated:
              Resolved: