Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.2
    • Fix Version/s: 2.1.5, 2.2.2
    • Component/s: Themes
    • Labels:
    • Testing Instructions:
      Hide
      1. Login as Admin and select Sky Hight theme
      2. Go to a course where a QUIZ has been completed and submitted for grading.
      3. Click on Review and you should see the last completed quiz questions/feedback/and number of correctly answered questions.
      4. In your browser window click on File/Print and choose Print as PDF/ or Print Preview, you should see a number of pages with contents of the whole of the Review page
      5. Go back to QUIZ page and in Navigation block select Quiz/Results/Grade
      6. Set report preference to "YES" in 'show/download marks for each question'
      7. Test to see that the grade table container scrolls horizontally enough to see the whole table contents.
      8. Feel free to test other types of report pages
      9. Make sure you test 2.1+, 2.2+ and master the changes differ because of conflicts.
      Show
      Login as Admin and select Sky Hight theme Go to a course where a QUIZ has been completed and submitted for grading. Click on Review and you should see the last completed quiz questions/feedback/and number of correctly answered questions. In your browser window click on File/Print and choose Print as PDF/ or Print Preview, you should see a number of pages with contents of the whole of the Review page Go back to QUIZ page and in 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. Feel free to test other types of report pages Make sure you test 2.1+, 2.2+ and master the changes differ because of conflicts.
    • Workaround:
      Hide

      Not easy!

      Show
      Not easy!
    • Affected Branches:
      MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-31328_master
    • Rank:
      37799

      Description

      After making changes to Sky High report layout, in MDL-31189, the problem with truncated pages was fixed for some reports, but not others. Further investigation revealed the inadequacy of this theme's double right-handed blog-style layout, so more changes needed making to the report layout and css to fix this. Working more and more with layouts, and trying different styles, Dietmar and I, have come to the conclusion that the percentage layout works best, it uses less layers, and yet appears to have a more ridged structure.

      1. 31328.screenshot.png
        52 kB
      2. chrome_print_preview.png
        52 kB
      3. FF_print_preview.png
        124 kB
      4. MDL-31328-skyhigh.jpg
        69 kB

        Issue Links

          Activity

          Hide
          Mary Evans added a comment -

          I have these files ready, but need to wait to see if MDL-31189 gets pulled this week, as I need this to be in place before I apply the changes in this tracker issue, which ultimately will make Sky High work much better, and pave the way for a different report layout for CORE themes overall.

          Hopefully so at least!

          Show
          Mary Evans added a comment - I have these files ready, but need to wait to see if MDL-31189 gets pulled this week, as I need this to be in place before I apply the changes in this tracker issue, which ultimately will make Sky High work much better, and pave the way for a different report layout for CORE themes overall. Hopefully so at least!
          Hide
          Aaron Webster added a comment -

          I've been talking to Mary the last couple of days via email and she has asked me to log this information. Here goes:

          Problem: Teachers need to be able to print quiz attempts to PDF -> currently they are truncated

          Mary asked that I try the latest release of Sky High. I downloaded it, and installed on development Moodle. Quiz attempts were still being truncated when printed to PDF.

          Mary sent me her latest working copy of Sky High. The problem remained, but I made some changes that 'fixed' it.

          In config.php, I changed line 113 from "'file' => 'general.php'," to "'file' => 'report.php',", my reasoning being that the 'report.php' layout contained the fix. This actually fixed the quiz truncation, but brought to light another bug. reports should not be 'incourse', and this is what is happening.

          Show
          Aaron Webster added a comment - I've been talking to Mary the last couple of days via email and she has asked me to log this information. Here goes: Problem: Teachers need to be able to print quiz attempts to PDF -> currently they are truncated Mary asked that I try the latest release of Sky High. I downloaded it, and installed on development Moodle. Quiz attempts were still being truncated when printed to PDF. Mary sent me her latest working copy of Sky High. The problem remained, but I made some changes that 'fixed' it. In config.php, I changed line 113 from "'file' => 'general.php'," to "'file' => 'report.php',", my reasoning being that the 'report.php' layout contained the fix. This actually fixed the quiz truncation, but brought to light another bug. reports should not be 'incourse', and this is what is happening.
          Hide
          Mary Evans added a comment - - edited

          Hi Aaron,
          I don't understand why the QUIZ review is causing this problem at all. It just does not make sense. From what I can gather, it must be the way this particular page is set up, and for some reason not setting pagelayout-report in the body tag. If it did then the report page would be used by default. So that is probably why you got it to work they way you wanted it to work.

          I'm aiming to start working on this and try and get it in got integration ASAP.

          Thanks
          Mary

          Show
          Mary Evans added a comment - - edited Hi Aaron, I don't understand why the QUIZ review is causing this problem at all. It just does not make sense. From what I can gather, it must be the way this particular page is set up, and for some reason not setting pagelayout-report in the body tag. If it did then the report page would be used by default. So that is probably why you got it to work they way you wanted it to work. I'm aiming to start working on this and try and get it in got integration ASAP. Thanks Mary
          Hide
          Sam Hemelryk added a comment -

          Hi Mary,

          I've been having a look at this but it looks like it is a long way from complete sorry.
          The following at the things I noted:

          • report.php you've added $hassidepost as a check however that variable is not defined, and doesn't look like it needs to be checked.
          • When viewing a course and the site frontpage there is a white page at the top before the page starts, and at the bottom as well where the footer starts. (comparison before and after shows it up clearly)
          • Admin pages are really messed up. Blocks at bottom etc, I'll attach a screenshot of this.

          Really the first point is very minor only leading to a notice under some circumstances, however the 2nd and 3rd points are larger problems. Admin pages appear to be totally busted with this patch.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi Mary, I've been having a look at this but it looks like it is a long way from complete sorry. The following at the things I noted: report.php you've added $hassidepost as a check however that variable is not defined, and doesn't look like it needs to be checked. When viewing a course and the site frontpage there is a white page at the top before the page starts, and at the bottom as well where the footer starts. (comparison before and after shows it up clearly) Admin pages are really messed up. Blocks at bottom etc, I'll attach a screenshot of this. Really the first point is very minor only leading to a notice under some circumstances, however the 2nd and 3rd points are larger problems. Admin pages appear to be totally busted with this patch. Cheers Sam
          Hide
          Mary Evans added a comment - - edited

          Hi Sam,
          I don't know why your version of Moodle is displaying the theme in that way as it looks perfectly good in Last weekly update? Please see my screenshot.

          I don't have my screen resolution set wide, so I use zoom in/out to achieve the same page view, and what I did see is that the footer is missing background color, which I don't think you picked up on.

          However, your screenshot looks very odd and as I say nothing like my version.

          If this is what Moodle 2.3 dev has done to Sky High then there is something wrong with Moodle 2.3. Have you checked other themes? It's very strange!

          Did you test the truncating page issue which is what this is all about fixing?

          Cheers

          Show
          Mary Evans added a comment - - edited Hi Sam, I don't know why your version of Moodle is displaying the theme in that way as it looks perfectly good in Last weekly update? Please see my screenshot. I don't have my screen resolution set wide, so I use zoom in/out to achieve the same page view, and what I did see is that the footer is missing background color, which I don't think you picked up on. However, your screenshot looks very odd and as I say nothing like my version. If this is what Moodle 2.3 dev has done to Sky High then there is something wrong with Moodle 2.3. Have you checked other themes? It's very strange! Did you test the truncating page issue which is what this is all about fixing? Cheers
          Hide
          Dietmar Wagner added a comment -

          Hi Sam,

          I just tested sky_high on latest 2.3dev (Build: 20120213) and can't reproduce your problem. Everything is at it's palce and works fine (regardless wether I use Mary's max-width patch or not).

          As theme sky_high uses report.php from base (config.php) and there is no check for $hassidepost there might be something wrong with your version of sky_high or moodle. Mary is right: very strange .

          Cheers
          Dietmar

          Show
          Dietmar Wagner added a comment - Hi Sam, I just tested sky_high on latest 2.3dev (Build: 20120213) and can't reproduce your problem. Everything is at it's palce and works fine (regardless wether I use Mary's max-width patch or not). As theme sky_high uses report.php from base (config.php) and there is no check for $hassidepost there might be something wrong with your version of sky_high or moodle. Mary is right: very strange . Cheers Dietmar
          Hide
          Mary Evans added a comment - - edited

          Hi Dietmar,

          In this final attempt for the truncated page problem, which this patch is addressing, I removed the link back to theme/base/layout/report.php in config.php and added the report we have been working on. I forgot to take out the reference to $hassidepost in the new report.php but that is, as Sam pointed out a minor detail.

          I'm going to do a full test on this again now and see if I get anything different.

          Thanks for your continued support in this project...

          Cheers
          Mary

          Show
          Mary Evans added a comment - - edited Hi Dietmar, In this final attempt for the truncated page problem, which this patch is addressing, I removed the link back to theme/base/layout/report.php in config.php and added the report we have been working on. I forgot to take out the reference to $hassidepost in the new report.php but that is, as Sam pointed out a minor detail. I'm going to do a full test on this again now and see if I get anything different. Thanks for your continued support in this project... Cheers Mary
          Hide
          Mary Evans added a comment - - edited

          Mia culpa! Mia culpa! mia MAXIMA culpa!

          I forgot to remove the reference to BASE theme in Admin layout in config.php

          I had done this in my working copy but forgot to correct this in my GIT MDL-31328_master branch.

          I've just added in a new commit, plus taken out the reference to $hassidepost in report.php, so hopefully you will see a vast difference.

          Cheers
          Mary

          Show
          Mary Evans added a comment - - edited Mia culpa! Mia culpa! mia MAXIMA culpa! I forgot to remove the reference to BASE theme in Admin layout in config.php I had done this in my working copy but forgot to correct this in my GIT MDL-31328 _master branch. I've just added in a new commit, plus taken out the reference to $hassidepost in report.php, so hopefully you will see a vast difference. Cheers Mary
          Hide
          Dietmar Wagner added a comment -

          C'est la vie!

          Dietmar

          Show
          Dietmar Wagner added a comment - C'est la vie! Dietmar
          Hide
          Sam Hemelryk added a comment -

          Hi guys,

          I've just gave this a quick test again now and indeed admin pages are working as expected now thanks

          I can still see the white bar at the top and bottom of the page, a quick play and it looks like the is being caused by margins being collapsed.
          Margin collapsing is a fun one, I know a little about it but had never dug into depth about this type of issue until now.
          Reading through the W3 spec for the box model I found what I believe to be the problem:

          The top margin of an in-flow block element collapses with its first in-flow block-level child's top margin if the element has no top border, no top padding, and the child has no clearance.

          In this case it appears #page is collapsing the margin with its first in flow child #report-wrapper (which has margin-top:20px).
          Adding ''padding-top: 1px'' or a border to #page (or likely the body element as well) will fix this issue.

          I am experiencing this problem in both Firefox (10.0.1) and Chrome (17.0.963.46) btw.

          Keen to get your feedback on this one.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi guys, I've just gave this a quick test again now and indeed admin pages are working as expected now thanks I can still see the white bar at the top and bottom of the page, a quick play and it looks like the is being caused by margins being collapsed. Margin collapsing is a fun one, I know a little about it but had never dug into depth about this type of issue until now. Reading through the W3 spec for the box model I found what I believe to be the problem: The top margin of an in-flow block element collapses with its first in-flow block-level child's top margin if the element has no top border, no top padding, and the child has no clearance. In this case it appears #page is collapsing the margin with its first in flow child #report-wrapper (which has margin-top:20px). Adding ''padding-top: 1px'' or a border to #page (or likely the body element as well) will fix this issue. I am experiencing this problem in both Firefox (10.0.1) and Chrome (17.0.963.46) btw. Keen to get your feedback on this one. Cheers Sam
          Hide
          Sam Hemelryk added a comment -

          Here's the link to the spec http://www.w3.org/TR/CSS2/box.html#collapsing-margins (jumps straight to collapsing margins) if you're keen to have a read

          Show
          Sam Hemelryk added a comment - Here's the link to the spec http://www.w3.org/TR/CSS2/box.html#collapsing-margins (jumps straight to collapsing margins) if you're keen to have a read
          Hide
          Mary Evans added a comment - - edited

          You may well be right Sam, but I think it's more to do with the way the pages are styled. If you add the background for the page into html, body rather than in a set of separate pages as it is at present, the white bar goes.

          Also one more fix for the body tag where fonts are set, this breaks YUI Font CSS as discussed in the Themes Forum http://moodle.org/mod/forum/discuss.php?d=162731#p712890 started 22 Nov 2010 by Urs Hunkler which was a long while back which lead to Patrick creating MDL-25512 to fix it in Canvas theme, which I fixed in April last year!

          Here is what I propose...

          theme/sky_high/style/core.css
          
          /* ADDED BACKGROUND IMAGE TO HTML, BODY
          ----------------------------------------*/
          html, body {
              background:url([[pix:theme|body]]) top left repeat-x #dcecf9;
          }
          
          /* REMOVE BODY AND ADDED PAGE TO THIS LINE SO AS NOT TO BREAK YUI FONT CSS
          ---------------------------------------------------------------------------*/
          
          page,h1,h2,h3,h4,h5,h6,p,ul,ol,dl,input,textarea {
              font-family:Arial, Helvetica, sans-serif;
              color:#333;
          }
          

          What do you think?

          Show
          Mary Evans added a comment - - edited You may well be right Sam, but I think it's more to do with the way the pages are styled. If you add the background for the page into html, body rather than in a set of separate pages as it is at present, the white bar goes. Also one more fix for the body tag where fonts are set, this breaks YUI Font CSS as discussed in the Themes Forum http://moodle.org/mod/forum/discuss.php?d=162731#p712890 started 22 Nov 2010 by Urs Hunkler which was a long while back which lead to Patrick creating MDL-25512 to fix it in Canvas theme, which I fixed in April last year! Here is what I propose... theme/sky_high/style/core.css /* ADDED BACKGROUND IMAGE TO HTML, BODY ----------------------------------------*/ html, body { background:url([[pix:theme|body]]) top left repeat-x #dcecf9; } /* REMOVE BODY AND ADDED PAGE TO THIS LINE SO AS NOT TO BREAK YUI FONT CSS ---------------------------------------------------------------------------*/ page,h1,h2,h3,h4,h5,h6,p,ul,ol,dl,input,textarea { font-family:Arial, Helvetica, sans-serif; color:#333; } What do you think?
          Hide
          Dietmar Wagner added a comment -

          Hi Mary,
          sounds good.
          But I can't really help in this situation. Whatever I do: no white bars!!??

          Dietmar

          Show
          Dietmar Wagner added a comment - Hi Mary, sounds good. But I can't really help in this situation. Whatever I do: no white bars!!?? Dietmar
          Hide
          Mary Evans added a comment -

          I forgot this was still here! I thought I had finished it and put it in for integration!!!

          Show
          Mary Evans added a comment - I forgot this was still here! I thought I had finished it and put it in for integration!!!
          Hide
          Mary Evans added a comment -

          I've only submitted one branch so far, but should be easy enough to cherry pick if the weather stays fine.

          Show
          Mary Evans added a comment - I've only submitted one branch so far, but should be easy enough to cherry pick if the weather stays fine.
          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 -

          rebased

          Show
          Mary Evans added a comment - rebased
          Hide
          Sam Hemelryk added a comment -

          Hi Mary,

          This is going to be integrated this week I've reviewed and am happy with it.
          However before it goes in I want to check, is this going to master only or is it going to the stable branches 2.2 and 2.1 as well?

          Once I hear back from you I'll integrate this

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi Mary, This is going to be integrated this week I've reviewed and am happy with it. However before it goes in I want to check, is this going to master only or is it going to the stable branches 2.2 and 2.1 as well? Once I hear back from you I'll integrate this Cheers Sam
          Hide
          Sam Hemelryk added a comment -

          Whoops forgot to click start review sorry.

          Show
          Sam Hemelryk added a comment - Whoops forgot to click start review sorry.
          Hide
          Mary Evans added a comment -

          Hi Sam,
          Yes please do cherry-pick this to branches 2.2 and 2.1 thanks.

          Show
          Mary Evans added a comment - Hi Sam, Yes please do cherry-pick this to branches 2.2 and 2.1 thanks.
          Hide
          Sam Hemelryk added a comment -

          Thanks Mary, I've merged it to master and backported to 2.1 and 2.2.
          Please in the future produce branches for the versions you want it applied to. In this case there were conflicts that I had to resolve in order to backport.
          I will update testing instructions shortly to explicitly test all branches.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Thanks Mary, I've merged it to master and backported to 2.1 and 2.2. Please in the future produce branches for the versions you want it applied to. In this case there were conflicts that I had to resolve in order to backport. I will update testing instructions shortly to explicitly test all branches. Cheers Sam
          Hide
          Mary Evans added a comment -

          Thanks Sam,
          Sorry that you had those conflicts to deal with. I had ever good intention to submit the branches this afternoon but fell asleep instead, and so missed the opportunity today to do them.

          Show
          Mary Evans added a comment - Thanks Sam, Sorry that you had those conflicts to deal with. I had ever good intention to submit the branches this afternoon but fell asleep instead, and so missed the opportunity today to do them.
          Hide
          Rossiani Wijaya added a comment -

          Hi Mary,

          While testing this, I noticed when 'Show / download marks for each question' is set to yes, the output display on print preview is still truncate. However it is fine if it is set to no.

          Could you confirm whether this is still a bug or intentional?

          *attaching screenshot on FF and Chrome.

          Show
          Rossiani Wijaya added a comment - Hi Mary, While testing this, I noticed when 'Show / download marks for each question' is set to yes, the output display on print preview is still truncate. However it is fine if it is set to no. Could you confirm whether this is still a bug or intentional? *attaching screenshot on FF and Chrome.
          Hide
          Rossiani Wijaya added a comment -

          Created follow up issue (MDL-31954) for truncates report when printing pdf/print preview.

          I'm passing this issue because it fixed the major issue for displaying the report properly on webpage.

          Test passed.

          Show
          Rossiani Wijaya added a comment - Created follow up issue ( MDL-31954 ) for truncates report when printing pdf/print preview. I'm passing this issue because it fixed the major issue for displaying the report properly on webpage. Test passed.
          Hide
          Mary Evans added a comment -

          Hi Rossiani,

          That page was NOT in the test for printing. The page asked for was only the review page. The page you have in the image you uploaded is quiz/results/grade. The only thing to test here was the horizontal scroll.

          But I will take a look at MDL-31954 and see what the problem is there.

          I must admit this theme is fast becoming nightmare to get right.

          I feel like kicking it 'sky high' some days! LOL

          Show
          Mary Evans added a comment - Hi Rossiani, That page was NOT in the test for printing. The page asked for was only the review page. The page you have in the image you uploaded is quiz/results/grade. The only thing to test here was the horizontal scroll. But I will take a look at MDL-31954 and see what the problem is there. I must admit this theme is fast becoming nightmare to get right. I feel like kicking it 'sky high' some days! LOL
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Your changes are now upstream and will be included in the next minor released scheduled for March 13th (next Monday!).

          icao_reverse('arreis olik rebemevon afla letoh ognat');
          

          Closing, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Your changes are now upstream and will be included in the next minor released scheduled for March 13th (next Monday!). icao_reverse('arreis olik rebemevon afla letoh ognat'); Closing, ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: