Moodle
  1. Moodle
  2. MDL-31345

Afterburner horizontal scroll in report

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.1.4, 2.2.1, 2.3
    • Fix Version/s: 2.1.5, 2.2.2
    • Component/s: Quiz, Reports, Themes
    • Labels:
    • Environment:
      server linux, apache
    • Testing Instructions:
      Hide

      Since this patch aims to fix pagelayout-report in ../mod/quiz/report.php?id.. the tester needs to have a quiz which has been attempted by a student or students the more the merrier. Although I actually tested this with only one student who had made 2 attempts at one question, but there was sufficient info to scroll the page if the browser window was resized.(see screen-shot)

      1. Login as Admin select Afterburner theme in normal way.
      2. Go to a Quiz page and in the Navigation block select Quiz/Results/Grade
      3. Set report preference to "YES" in 'show/download marks for each question'
      4. Test to see that the grade table container scrolls horizontally enough to see the whole table contents.
      Show
      Since this patch aims to fix pagelayout-report in ../mod/quiz/report.php?id.. the tester needs to have a quiz which has been attempted by a student or students the more the merrier. Although I actually tested this with only one student who had made 2 attempts at one question, but there was sufficient info to scroll the page if the browser window was resized.(see screen-shot) Login as Admin select Afterburner theme in normal way. Go to a Quiz page and in the Navigation block select Quiz/Results/Grade Set report preference to "YES" in 'show/download marks for each question' Test to see that the grade table container scrolls horizontally enough to see the whole table contents.
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull Master Branch:
      MDL-31345_master

      Description

      When I login as an admin (or a teacher) and I view the grades for a quiz (/mod/quiz/report.php?id=...) the question responses on the far right are "chopped off"

        Gliffy Diagrams

        1. MDL-31345-QUIZ-PAGELAYOUT-REPORT.jpg
          33 kB
        2. report_cut.jpg
          604 kB
        3. safari_test.png
          141 kB

          Activity

          Hide
          Michael de Raadt added a comment -

          Thanks for reporting that.

          Have you tried this in another theme? It would be good to know if the problem is isolated to this theme or if it is a problem with the quiz report itself.

          Show
          Michael de Raadt added a comment - Thanks for reporting that. Have you tried this in another theme? It would be good to know if the problem is isolated to this theme or if it is a problem with the quiz report itself.
          Hide
          Ser Bal added a comment -

          Hello, I've tried all themes and the problem is is isolated to this theme "Afterburner"

          Show
          Ser Bal added a comment - Hello, I've tried all themes and the problem is is isolated to this theme "Afterburner"
          Hide
          Scott Karren added a comment -

          I have tried different themes and have run into the same issue. You can look at http://moodle.org/mod/forum/discuss.php?d=195180 for more information.

          Show
          Scott Karren added a comment - I have tried different themes and have run into the same issue. You can look at http://moodle.org/mod/forum/discuss.php?d=195180 for more information.
          Hide
          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
          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
          Rob added a comment -

          I am on the latest 2.1.4+ (from last Saturday), and the issue affects many core themes. It is not just Afterburner.

          Brick, Fusion, Magazine, Nonzero, Nimble and Overlay have the scrollbar.

          Anomaly, Standard, Boxxie, Leatherbound, Binarius, Arialist, Sky High and Splash do not.

          I have some contributed themes installed as well, but I did not check those. I may not have the latest release.

          Show
          Rob added a comment - I am on the latest 2.1.4+ (from last Saturday), and the issue affects many core themes. It is not just Afterburner. Brick, Fusion, Magazine, Nonzero, Nimble and Overlay have the scrollbar. Anomaly, Standard, Boxxie, Leatherbound, Binarius, Arialist, Sky High and Splash do not. I have some contributed themes installed as well, but I did not check those. I may not have the latest release.
          Hide
          Mary Evans added a comment -

          Hi Rob,

          Thanks for testing those themes. Looking at this it is probably something that could be done in the Mod itself. I'll have to investigate this.

          Cheers
          Mary

          Show
          Mary Evans added a comment - Hi Rob, Thanks for testing those themes. Looking at this it is probably something that could be done in the Mod itself. I'll have to investigate this. Cheers Mary
          Hide
          Mary Evans added a comment -

          I've just halted the integration of this for the time being while I do some more testing.

          Show
          Mary Evans added a comment - I've just halted the integration of this for the time being while I do some more testing.
          Hide
          Mary Evans added a comment - - edited

          Ok...I've made some changed to the original patch for this issue, so the bottom scroll bar, in the browser, scrolls as far as needed for any report page. This is far better than the previous fix. The beauty of this method is that you have the scroll bar available to you, at whatever depth of page you are at, you don't need to go hunting for it, as you would have had to do in my previous fix.

          As for the other themes I might add them to this as sub-tasks...but not sure yet as there is quite a bit of work still ongoing with pagelayout-report in general in another tracker issue.

          That's it done now...so all we can do now is wait.

          Cheers
          Mary

          Show
          Mary Evans added a comment - - edited Ok...I've made some changed to the original patch for this issue, so the bottom scroll bar, in the browser, scrolls as far as needed for any report page. This is far better than the previous fix. The beauty of this method is that you have the scroll bar available to you, at whatever depth of page you are at, you don't need to go hunting for it, as you would have had to do in my previous fix. As for the other themes I might add them to this as sub-tasks...but not sure yet as there is quite a bit of work still ongoing with pagelayout-report in general in another tracker issue. That's it done now...so all we can do now is wait. Cheers Mary
          Hide
          omer added a comment -

          Hey mary.

          the latest fix didnt work for me (Hebrew, RTL),
          while the first fix

          .pagelayout-report #page-content #region-main-box #region-post-box #region-main-wrap #region-main-pad #region-main .region-content .no-overflow

          {overflow: auto;}

          wokres perfectly !

          Show
          omer added a comment - Hey mary. the latest fix didnt work for me (Hebrew, RTL), while the first fix .pagelayout-report #page-content #region-main-box #region-post-box #region-main-wrap #region-main-pad #region-main .region-content .no-overflow {overflow: auto;} wokres perfectly !
          Hide
          Eloy Lafuente (stronk7) added a comment -

          I bet this must go back to 22 and 21 stables too, correct?

          Also... about omer's comment above ... should we halt this until proved to work RTL?

          Ciao

          PS: Note to integrators... this contains 4-5 commits that ideally should be joined into one.

          Show
          Eloy Lafuente (stronk7) added a comment - I bet this must go back to 22 and 21 stables too, correct? Also... about omer's comment above ... should we halt this until proved to work RTL? Ciao PS: Note to integrators... this contains 4-5 commits that ideally should be joined into one.
          Hide
          Mary Evans added a comment -

          Eloy, I haven't quite got to grips with GIT so I am not sure how I should approach combining all the commits my changes to this patch created. I need to go to back to school to learn all this.

          I'm not totally convinced about the RTL as the patch adds a scroll bar to the page not just to the table as Omar prefers.

          Sam was not too keen on the first method, that's one of the reasons I dropped it in favour of the one which other theme use.

          So this said... I would prefer this to go through now and if possible cherry-pick to 22 and 21 stable please. If you would prefer me to redo this as one commit I can.
          Cheers

          Show
          Mary Evans added a comment - Eloy, I haven't quite got to grips with GIT so I am not sure how I should approach combining all the commits my changes to this patch created. I need to go to back to school to learn all this. I'm not totally convinced about the RTL as the patch adds a scroll bar to the page not just to the table as Omar prefers. Sam was not too keen on the first method, that's one of the reasons I dropped it in favour of the one which other theme use. So this said... I would prefer this to go through now and if possible cherry-pick to 22 and 21 stable please. If you would prefer me to redo this as one commit I can. Cheers
          Hide
          Mary Evans added a comment - - edited

          I have just re-worked this and cleaned up the commits, and also added fix for RTL in rtl.css which should fix the RTL scroll problem too.

          Just so you know I know, after making sure all was saved in correct formal (UNIX UTF-8) I got a warning that LF would now become CRLF is this going to cause me a problem, Eloy?

          Show
          Mary Evans added a comment - - edited I have just re-worked this and cleaned up the commits, and also added fix for RTL in rtl.css which should fix the RTL scroll problem too. Just so you know I know, after making sure all was saved in correct formal (UNIX UTF-8) I got a warning that LF would now become CRLF is this going to cause me a problem, Eloy?
          Hide
          Aparup Banerjee added a comment -

          Hi Mary, to combine commits you can try a 'git rebase -i' which is an interactive rebase. You should be prompted with an editor with some comments about how to proceed.

          Generally, i've had to pick 's' for squashing commits, and then the specified commits would be combined (squashed) and a whole new commit message could be written to describe them all

          Show
          Aparup Banerjee added a comment - Hi Mary, to combine commits you can try a 'git rebase -i' which is an interactive rebase. You should be prompted with an editor with some comments about how to proceed. Generally, i've had to pick 's' for squashing commits, and then the specified commits would be combined (squashed) and a whole new commit message could be written to describe them all
          Hide
          Mary Evans added a comment - - edited

          Hi Aparup,

          I have just redone them and added stable branches too and cherry picking along the way!

          Show
          Mary Evans added a comment - - edited Hi Aparup, I have just redone them and added stable branches too and cherry picking along the way!
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Hey, you're becoming a git-pro! squashing, cherry-picking... congrats! :-P

          About to integrate this now...

          Show
          Eloy Lafuente (stronk7) added a comment - Hey, you're becoming a git-pro! squashing, cherry-picking... congrats! :-P About to integrate this now...
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Re: About cr/lf... I don't know why/where you did get that message, but it seems that your patch contains correct lf ones, so no problem. Which OS and environment (IDE) do you use for coding all this stuff?

          Show
          Eloy Lafuente (stronk7) added a comment - Re: About cr/lf... I don't know why/where you did get that message, but it seems that your patch contains correct lf ones, so no problem. Which OS and environment (IDE) do you use for coding all this stuff?
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Integrated (21, 22 and master), thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Integrated (21, 22 and master), thanks!
          Hide
          Mary Evans added a comment -

          Thanks Eloy!

          As for OS & IDE (whatever that means)
          Windows 7
          GIT
          TextPad (PHP & CSS set to UNIX UTF-8)

          http://www.textpad.com/products/textpad/index.html

          I got the message in GIT Bash when I came to do

          git add file

          prior to doing the commit.

          Show
          Mary Evans added a comment - Thanks Eloy! As for OS & IDE (whatever that means) Windows 7 GIT TextPad (PHP & CSS set to UNIX UTF-8) http://www.textpad.com/products/textpad/index.html I got the message in GIT Bash when I came to do git add file prior to doing the commit.
          Hide
          Rob added a comment -

          Hi Mary,

          Is your fix only for Afterburner, or will the other themes be fixed too?

          Rob

          Show
          Rob added a comment - Hi Mary, Is your fix only for Afterburner, or will the other themes be fixed too? Rob
          Hide
          Mary Evans added a comment -

          Hi Rob,
          This is an Afterburner fix ONLY.

          Are other themes affected? I thought they were OK...if not then please can you report the themes affected in a new Moodle Tracker issue?

          Thanks
          Mary

          Show
          Mary Evans added a comment - Hi Rob, This is an Afterburner fix ONLY. Are other themes affected? I thought they were OK...if not then please can you report the themes affected in a new Moodle Tracker issue? Thanks Mary
          Hide
          Rob added a comment -

          Below is the list from my previous comment in this thread:

          Brick, Fusion, Magazine, Nonzero, Nimble and Overlay have the scrollbar.

          Anomaly, Standard, Boxxie, Leatherbound, Binarius, Arialist, Sky High and Splash do not.

          We are using Sky High. My teachers use that report extensivly. There was a scrollbar in the past, but it went away in a recent update. In addition, we allow course themes, so fixing the others would be helpful too. Thank you for all your help.

          Rob

          Show
          Rob added a comment - Below is the list from my previous comment in this thread: Brick, Fusion, Magazine, Nonzero, Nimble and Overlay have the scrollbar. Anomaly, Standard, Boxxie, Leatherbound, Binarius, Arialist, Sky High and Splash do not. We are using Sky High. My teachers use that report extensivly. There was a scrollbar in the past, but it went away in a recent update. In addition, we allow course themes, so fixing the others would be helpful too. Thank you for all your help. Rob
          Hide
          Mary Evans added a comment -

          Ah...sorry I forget which day it is let alone which bug I am working on! LOL

          I have this now...and will probably get this sorted out for next week's pull.

          Show
          Mary Evans added a comment - Ah...sorry I forget which day it is let alone which bug I am working on! LOL I have this now...and will probably get this sorted out for next week's pull.
          Hide
          Mary Evans added a comment -

          I have just created MDL-31566 in an attempt to fix this in other CORE themes too.

          Show
          Mary Evans added a comment - I have just created MDL-31566 in an attempt to fix this in other CORE themes too.
          Hide
          Ankit Agarwal added a comment -

          Hi Mary,
          I tried to test this, but the column still appears chopped off for me. This can be due to other window specific warnings(Already existing issue -invalid CRT parameters) messing with the page layout on my end. I will wait till someone on lamp takes this up and gives it a try.
          Thanks

          Show
          Ankit Agarwal added a comment - Hi Mary, I tried to test this, but the column still appears chopped off for me. This can be due to other window specific warnings(Already existing issue -invalid CRT parameters) messing with the page layout on my end. I will wait till someone on lamp takes this up and gives it a try. Thanks
          Hide
          Mary Evans added a comment -

          Hi Ankit,

          Any progress with this?
          All the fix does is adds a scroll bar to the bottom of the browser window, and if you use a RTL language the actual table on the page gets a scroll bar too (or should!)

          Cheers
          Mary

          Show
          Mary Evans added a comment - Hi Ankit, Any progress with this? All the fix does is adds a scroll bar to the bottom of the browser window, and if you use a RTL language the actual table on the page gets a scroll bar too (or should!) Cheers Mary
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Hi,

          I'm attaching one screen-shot of how the table and the window looks after applying the patch, exactly the same happens both with Safari and Firefox here:

          1) The table continues not showing any scroll-bar, so contents are cut.

          2) The window has one horizontal scroll-bar, but using it only leads to blank contents. Only making the window BIG (1900) I can see both the table and the graph properly.

          So I'm going to revert and reopen this issue. Clearly I'm not getting the scroll bar in the table as expected.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Hi, I'm attaching one screen-shot of how the table and the window looks after applying the patch, exactly the same happens both with Safari and Firefox here: 1) The table continues not showing any scroll-bar, so contents are cut. 2) The window has one horizontal scroll-bar, but using it only leads to blank contents. Only making the window BIG (1900) I can see both the table and the graph properly. So I'm going to revert and reopen this issue. Clearly I'm not getting the scroll bar in the table as expected. Ciao
          Hide
          Dietmar Wagner added a comment -

          Hi Mary,

          in my eyes we only have to add the following line to afterburner-layout.css for achieving the horizontal scrollbar in the right way:

          .pagelayout-report #report-page-content #report-region-main-box #report-region-post-box #report-region-main-wrap #report-region-main .region-content

          {overflow-x: auto;}

          [tested with IE8/9, Firefox and Chrome + ltr/rtl].

          "My" IE7 seems to be unable to handle afterburner in combination with rtl languages!?

          Cheers
          Dietmar

          Show
          Dietmar Wagner added a comment - Hi Mary, in my eyes we only have to add the following line to afterburner-layout.css for achieving the horizontal scrollbar in the right way: .pagelayout-report #report-page-content #report-region-main-box #report-region-post-box #report-region-main-wrap #report-region-main .region-content {overflow-x: auto;} [tested with IE8/9, Firefox and Chrome + ltr/rtl] . "My" IE7 seems to be unable to handle afterburner in combination with rtl languages!? Cheers Dietmar
          Hide
          Dietmar Wagner added a comment -

          Test with Safari 5.1.2 and rtl/ltr: ok

          Show
          Dietmar Wagner added a comment - Test with Safari 5.1.2 and rtl/ltr: ok
          Hide
          Mary Evans added a comment - - edited

          Hi Dietmar,

          Afterburner doesn't use the report page layout, it uses default.php.
          I'm just wondering if you are testing Sky High as we have been working on that too, and there is a horizontal scroll problem on that theme also. So if this is the report of your testing Sky High that's great! Thanks

          I am still struggling with Afterburner as it worked well when I tested it last week now it's all over the place!

          This is what I had submitted which has been rejected, as it was not working the way I thought it would.

          .pagelayout-report #page-content {float:none; width:auto;}
           
          .pagelayout-report #page-content,
          .pagelayout-report #page-content #region-main-box,
          .pagelayout-report #page-content #region-main-box #region-post-box,
          .pagelayout-report #page-content #region-main-box #region-post-box #region-main-wrap #region-main-pad,
          .pagelayout-report #page-content #region-main-box #region-post-box #region-main-wrap #region-main-pad #region-main {overflow:visible;}
          

          Show
          Mary Evans added a comment - - edited Hi Dietmar, Afterburner doesn't use the report page layout, it uses default.php. I'm just wondering if you are testing Sky High as we have been working on that too, and there is a horizontal scroll problem on that theme also. So if this is the report of your testing Sky High that's great! Thanks I am still struggling with Afterburner as it worked well when I tested it last week now it's all over the place! This is what I had submitted which has been rejected, as it was not working the way I thought it would. .pagelayout-report #page-content {float:none; width:auto;}   .pagelayout-report #page-content, .pagelayout-report #page-content #region-main-box, .pagelayout-report #page-content #region-main-box #region-post-box, .pagelayout-report #page-content #region-main-box #region-post-box #region-main-wrap #region-main-pad, .pagelayout-report #page-content #region-main-box #region-post-box #region-main-wrap #region-main-pad #region-main {overflow:visible;}
          Hide
          Mary Evans added a comment -

          I finally got it back to...

          .pagelayout-report #page-content #region-main-box #region-post-box #region-main-wrap #region-main-pad #region-main .region-content .no-overflow {overflow: auto;}
          

          which works OK in...
          FF (v.10)
          Opera (11.52)
          Chrome (v.16.0.912.77 m)
          IE9

          Show
          Mary Evans added a comment - I finally got it back to... .pagelayout-report #page-content #region-main-box #region-post-box #region-main-wrap #region-main-pad #region-main .region-content .no-overflow {overflow: auto;} which works OK in... FF (v.10) Opera (11.52) Chrome (v.16.0.912.77 m) IE9
          Hide
          Mary Evans added a comment -

          @Aparup

          I tried out the git rebase -i and got into all kinds of problems.
          I am still wondering where VIM editor came from...what an antique of a thing that is! I couldn't quite grasp what I was supposed to do, but I did find some GIT documentation which explains it.

          But thanks just the same

          Show
          Mary Evans added a comment - @Aparup I tried out the git rebase -i and got into all kinds of problems. I am still wondering where VIM editor came from...what an antique of a thing that is! I couldn't quite grasp what I was supposed to do, but I did find some GIT documentation which explains it. But thanks just the same
          Hide
          Dietmar Wagner added a comment -

          @Mary: I'll test that with a clean theme installation and be back this afternoon!

          Show
          Dietmar Wagner added a comment - @Mary: I'll test that with a clean theme installation and be back this afternoon!
          Hide
          Dietmar Wagner added a comment -

          @Mary: afterburner pagelayout for reports works fine now!

          Will have a look at sky_high later!

          Cheers
          Dietmar

          Show
          Dietmar Wagner added a comment - @Mary: afterburner pagelayout for reports works fine now! Will have a look at sky_high later! Cheers Dietmar
          Hide
          Aparup Banerjee added a comment -

          Hi Mary
          sorry you ran into such trouble with vim, it can be really painful. Perhaps with textpad and windows you can look at how this guy made Git use textpad instead of vim with one setting.

          Show
          Aparup Banerjee added a comment - Hi Mary sorry you ran into such trouble with vim, it can be really painful. Perhaps with textpad and windows you can look at how this guy made Git use textpad instead of vim with one setting .
          Hide
          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
          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
          Mary Evans added a comment -

          @Aparup

          You are a STAR! Thanks for that...works great! I can do commit -a yeah! I never quite knew what that did till now! saves lode of time LOL
          I must try the squash one now I know what I am doing!

          Thanks

          Show
          Mary Evans added a comment - @Aparup You are a STAR! Thanks for that...works great! I can do commit -a yeah! I never quite knew what that did till now! saves lode of time LOL I must try the squash one now I know what I am doing! Thanks
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Integrated thanks!

          Note: Why is that theme soooo strange that it never shows the window horizontal scroll bar? Any page you try, for example, course page, if reduced horizontally looks really horrible.

          Show
          Eloy Lafuente (stronk7) added a comment - Integrated thanks! Note: Why is that theme soooo strange that it never shows the window horizontal scroll bar? Any page you try, for example, course page, if reduced horizontally looks really horrible.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Tested under safari, chrome, firefox (all OS X). The table now shows the horizontal scroll bar if necessary.

          But, as commented above, other contents aren't shown properly as far as the windows never shows the scrollbar. For example the course page, or, in the same quiz report, the graph is wider and you only can get it by making the window wide enough. For your consideration.

          Anyway, this passes. Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Tested under safari, chrome, firefox (all OS X). The table now shows the horizontal scroll bar if necessary. But, as commented above, other contents aren't shown properly as far as the windows never shows the scrollbar. For example the course page, or, in the same quiz report, the graph is wider and you only can get it by making the window wide enough. For your consideration. Anyway, this passes. Ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Some changes to Moodle should be milestones in the project by themselves.

          This is not the case and your fix is not so important, but your collaboration is highly appreciated, thanks!

          Closing as fixed, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Some changes to Moodle should be milestones in the project by themselves. This is not the case and your fix is not so important, but your collaboration is highly appreciated, thanks! Closing as fixed, ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: