Moodle
  1. Moodle
  2. MDL-28491

Display the navigation panel on the summary page

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.1
    • Fix Version/s: 2.2
    • Component/s: Quiz
    • Labels:
    • Testing Instructions:
      Hide

      In as many themes as possible:
      1. Preview a quiz as a teacher and/or attempt it as a student.
      2. Make sure everything looks OK, particularly the summary page, and particularly the transitions from attempt <-> summary -> review.

      Show
      In as many themes as possible: 1. Preview a quiz as a teacher and/or attempt it as a student. 2. Make sure everything looks OK, particularly the summary page, and particularly the transitions from attempt <-> summary -> review.
    • Affected Branches:
      MOODLE_21_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE
    • Pull from Repository:
    • Pull Master Branch:
    • Rank:
      18148

      Description

      During a quiz attempt, users interact with three pages: mod/quiz/attempt.php, summary.php and review.php.

      attempt.php and review.php are always displayed with at least one block region, because the question navigation gets added as a fake block.

      Therefore, in some themes, there is a big visual jar when you go to and from the summary page. I think that is bad, but the only way round it I can think of is to add an invisible fake block to the summary page (and then display: none it).

        Issue Links

          Activity

          Hide
          Tim Hunt added a comment -

          Also, the logic for whether $PAGE->blocks->show_only_fake_blocks(); is called is not the same as on the attempt and review pages.

          Show
          Tim Hunt added a comment - Also, the logic for whether $PAGE->blocks->show_only_fake_blocks(); is called is not the same as on the attempt and review pages.
          Hide
          Tim Hunt added a comment -

          Sam, I am sure that what I am doing here is an evil hack.

          On the other hand, I need this, or possibly something less evil, to make the OU theme work. And, I can't think of a less evil way to get this result.

          Still, I would like to know what you think.

          Show
          Tim Hunt added a comment - Sam, I am sure that what I am doing here is an evil hack. On the other hand, I need this, or possibly something less evil, to make the OU theme work. And, I can't think of a less evil way to get this result. Still, I would like to know what you think.
          Hide
          Sam Hemelryk added a comment -

          Hi Tim,

          I've been having a look at this and most importantly at the end effect it has on the themes.
          First up the code - it looks fine, I'd suggest adding a comment to explain why a fake block is being added to the page without any content though.

          The second is the outcome on the page and this is where I think we need to look for a better solution.
          I should point out I'm not 100% sure what the end outcome is that you are looking for or what you are seeing go wrong with the page presently, perhaps some screenshots before and after your changes to illustrate the problem would help.
          What I've been doing is going through the core theme's 1 by 1 looking at what effect this change has on the summary page, the following is what I've found:

          Theme Before After Visual notes
          Standard . OR  
          Afterburner . . Still centre but there is now a clearly empty block region
          Anomaly . OR  
          Arialist OL OL  
          Base . OR  
          Binarius . OL  
          Boxxie . OR  
          Brick . OL  
          Formal White . OR Still centre but there is now a clearly empty block region

          I stopped at formal white as the pattern is pretty clear - the change is causing the content to offset because of that empty block region now being printed to the page.
          In theme's where the block region doesn't set a background colour this change leads to things appearing to shift to either the left or the right for no reason.
          In theme's where their is a background colour we are now ending up with a clearly visible block region that wasn't there before.
          I'll attach some screenshots, before and after for the standard and formal_white theme's.

          In thinking about this a bit more this is perhaps what you are after, and perhaps it is just that I don't like these changes.

          When I look at this I can't help but think this is something that should be fixed in the theme and not in the code.
          If you want to have block regions displayed for consistency regardless of whether there are blocks in them or not then I think the best solution would be to move the check in the theme's layout.
          For most themes this check is done before printing the block region, if you move it within the block region to just before the block content is displayed you will always have the block content printed.
          OF course this is a site wide (or layout wide more accurately) change, not just a quiz page change.

          An interesting idea, perhaps that would offer an alternative solution would be to allow a page to set fallback layout if the primary layout doesn't exist. This way a page (summary.php) could specify a pagelayout like 'quizsummary' and a fallback layout of 'standard' allowing a theme to really control the layout, or alternativily fallback to something that is going to exist if the theme doesn't.
          Of course you'd end up with more layouts which is not ideal.

          Anyway, in summary: perhaps more information about what you really want, and some screenshots for when I miss the point again would really help me in understanding why you want to make these changes.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi Tim, I've been having a look at this and most importantly at the end effect it has on the themes. First up the code - it looks fine, I'd suggest adding a comment to explain why a fake block is being added to the page without any content though. The second is the outcome on the page and this is where I think we need to look for a better solution. I should point out I'm not 100% sure what the end outcome is that you are looking for or what you are seeing go wrong with the page presently, perhaps some screenshots before and after your changes to illustrate the problem would help. What I've been doing is going through the core theme's 1 by 1 looking at what effect this change has on the summary page, the following is what I've found: Theme Before After Visual notes Standard . OR   Afterburner . . Still centre but there is now a clearly empty block region Anomaly . OR   Arialist OL OL   Base . OR   Binarius . OL   Boxxie . OR   Brick . OL   Formal White . OR Still centre but there is now a clearly empty block region I stopped at formal white as the pattern is pretty clear - the change is causing the content to offset because of that empty block region now being printed to the page. In theme's where the block region doesn't set a background colour this change leads to things appearing to shift to either the left or the right for no reason. In theme's where their is a background colour we are now ending up with a clearly visible block region that wasn't there before. I'll attach some screenshots, before and after for the standard and formal_white theme's. In thinking about this a bit more this is perhaps what you are after, and perhaps it is just that I don't like these changes. When I look at this I can't help but think this is something that should be fixed in the theme and not in the code. If you want to have block regions displayed for consistency regardless of whether there are blocks in them or not then I think the best solution would be to move the check in the theme's layout. For most themes this check is done before printing the block region, if you move it within the block region to just before the block content is displayed you will always have the block content printed. OF course this is a site wide (or layout wide more accurately) change, not just a quiz page change. An interesting idea, perhaps that would offer an alternative solution would be to allow a page to set fallback layout if the primary layout doesn't exist. This way a page (summary.php) could specify a pagelayout like 'quizsummary' and a fallback layout of 'standard' allowing a theme to really control the layout, or alternativily fallback to something that is going to exist if the theme doesn't. Of course you'd end up with more layouts which is not ideal. Anyway, in summary: perhaps more information about what you really want, and some screenshots for when I miss the point again would really help me in understanding why you want to make these changes. Cheers Sam
          Hide
          Sam Hemelryk added a comment -

          Sorry forgot to click Finish peer review this morning.

          Show
          Sam Hemelryk added a comment - Sorry forgot to click Finish peer review this morning.
          Hide
          Tim Hunt added a comment -

          You have been looking at the one page in isolation - and the thing is this page is not used it isolation. It is very closely related to attempt.php. And, the thing to think about is what happens as you move between the two. That is really the issue for themes with a visible block region.

          However, I agree that for themes with an invisible block region, what I have done is not acceptable.

          Perhaps the best thing to do is to keep the fake block that appears on the attempt.php and review.php pages, but make it show some useful content. That would include:

          1. User's photo, if that option is selected.
          2. Countdown timer, if that option is selected.
          3. Re-start preview button, for teacher previews.

          The problem is that all that is optional.

          David suggested also adding a summary like "3 of 10 questions are not complete" or "All questions complete, if you don't want to change any answers, submit now."

          Show
          Tim Hunt added a comment - You have been looking at the one page in isolation - and the thing is this page is not used it isolation. It is very closely related to attempt.php. And, the thing to think about is what happens as you move between the two. That is really the issue for themes with a visible block region. However, I agree that for themes with an invisible block region, what I have done is not acceptable. Perhaps the best thing to do is to keep the fake block that appears on the attempt.php and review.php pages, but make it show some useful content. That would include: 1. User's photo, if that option is selected. 2. Countdown timer, if that option is selected. 3. Re-start preview button, for teacher previews. The problem is that all that is optional. David suggested also adding a summary like "3 of 10 questions are not complete" or "All questions complete, if you don't want to change any answers, submit now."
          Hide
          Tim Hunt added a comment -

          OK. I thought better of this over the weekend. I will now try just displaying the standard navigation panel on the summary page, and see how that looks.

          I will probably leave this in testing here for the week, and then submit it for integration next week if we are happy it looks OK.

          Show
          Tim Hunt added a comment - OK. I thought better of this over the weekend. I will now try just displaying the standard navigation panel on the summary page, and see how that looks. I will probably leave this in testing here for the week, and then submit it for integration next week if we are happy it looks OK.
          Hide
          Tim Hunt added a comment -

          I think this is the right solution.

          This is effectively what we have been running for the last few months at the OU in our Moodle 2.x system (adding the fake_block via a stunningly evil hack in out theme's renderer rather than changing core code) and it looks and feels more natural.

          Show
          Tim Hunt added a comment - I think this is the right solution. This is effectively what we have been running for the last few months at the OU in our Moodle 2.x system (adding the fake_block via a stunningly evil hack in out theme's renderer rather than changing core code) and it looks and feels more natural.
          Hide
          Sam Hemelryk added a comment -

          Thanks Tim - this has been integrated now

          Show
          Sam Hemelryk added a comment - Thanks Tim - this has been integrated now
          Hide
          Rossiani Wijaya added a comment -

          The following notice message occurs:
          Notice: Undefined variable: page in /moodle/mod/quiz/summary.php on line 82

          It should probably use $PAGE instead of $page.

          Test failed.

          Show
          Rossiani Wijaya added a comment - The following notice message occurs: Notice: Undefined variable: page in /moodle/mod/quiz/summary.php on line 82 It should probably use $PAGE instead of $page. Test failed.
          Hide
          Tim Hunt added a comment -

          Why didn't that show up during my testing? Weird.

          Note that $PAGE not right. $page should be -1 (current page number within the quiz attempt).

          New commit https://github.com/timhunt/moodle/commit/0e2274c382d1ac0d0b8622ef1b23007e7cf00d4b added to the branch. Please re-integrate. Thanks.

          Show
          Tim Hunt added a comment - Why didn't that show up during my testing? Weird. Note that $PAGE not right. $page should be -1 (current page number within the quiz attempt). New commit https://github.com/timhunt/moodle/commit/0e2274c382d1ac0d0b8622ef1b23007e7cf00d4b added to the branch. Please re-integrate. Thanks.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          re-started testing phase, extra commit has been integrated. Thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - re-started testing phase, extra commit has been integrated. Thanks!
          Hide
          Rossiani Wijaya added a comment -

          This is working great.

          Thanks Tim for fixing it.

          Test passed.

          Show
          Rossiani Wijaya added a comment - This is working great. Thanks Tim for fixing it. Test passed.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Many thanks for all the hard work. This is now part of Moodle, your favorite LMS.

          Closing as fixed, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Many thanks for all the hard work. This is now part of Moodle, your favorite LMS. Closing as fixed, ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: