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

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

    Details

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

      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.
      4. TEST that user names (headings above tables) are centred and not misaligned.
      Show
      Testing: Go to Course Administration > Grades Select User Report from the drop down menu Select All Users option from the right menu. TEST that user names (headings above tables) are centred and not misaligned.
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull Master Branch:
      MDL-38657_master

      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.

        Gliffy Diagrams

          Activity

          Hide
          xan 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
          xan 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
          xan Prateek Sachan added a comment -

          Does this look good?

          Show
          xan Prateek Sachan added a comment - Does this look good?
          Hide
          dobedobedoh 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
          dobedobedoh 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
          dobedobedoh 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
          dobedobedoh 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
          lazydaisy 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
          lazydaisy 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
          lazydaisy 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
          lazydaisy 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
          xan 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
          xan 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
          lazydaisy 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
          lazydaisy 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
          poltawski Dan Poltawski added a comment -

          Integrated to 23 24 and master, thanks Mary

          Show
          poltawski Dan Poltawski added a comment - Integrated to 23 24 and master, thanks Mary
          Hide
          rajeshtaneja 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
          rajeshtaneja 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
          bawjaws 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
          bawjaws 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
          bawjaws 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
          bawjaws 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
          lazydaisy 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
          lazydaisy 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
          poltawski Dan Poltawski added a comment -

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

          (For the record I love crazy themers! )

          Show
          poltawski Dan Poltawski added a comment - Sending back to testing because I think this can be passed? (For the record I love crazy themers! )
          Hide
          rajeshtaneja 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
          rajeshtaneja 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
          poltawski 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
          poltawski 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:
                Fix Release Date:
                13/May/13