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
    • Rank:
      37821

      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"

      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: