Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.2
    • Fix Version/s: 2.3
    • Component/s: Course
    • Labels:
      None
    • Testing Instructions:
      Hide

      Test social format

      1. Create a course with format set to social
      2. Ensure the social format is working as expected

      Test scorm format

      1. Create a course with format set to scorm
      2. Upload a packaegthe social format is working as expected

      Test single page

      Setup:

      1. Create a course with course display setting set to 'single page'

      Topics:

      1. Set course format to topics
      2. Disable course ajax in Admin > Appearance > Ajax and Javascript
      3. Verify that the course appears as before change visually
      4. Turn editting on/off with the button in top right and verify you are returned to where you were
      5. Turn editting on/off with the button in the settings menu and verify you are returned to where you were
      6. Try adding a resource to a section (and you are returned to where you were)
      7. Try moving a resource between sections (and you are returned to where you were)
      8. Try indenting a section (and you are returned to where you were)
      9. Try editing a resource (and you are returned to where you were)
      10. Try hiding a resource (and you are returned to where you were)
      11. Try deleting a resource (and you are returned to where you were)
        # Try duplicating a resource (and you are returned to where you were)
      12. Try moving a section
      13. Try hiding a section
      14. Try setting a setting a section as the current topic and verify it has been marked visually
      15. Turn AJAX back on
      16. Try moving a resource between sections (turn editing off to verify this action has been completed)
      17. Try indenting a section (turn editing off to verify this action has been completed)
      18. Try hiding a resource (turn editing off to verify this action has been completed)
      19. Try deleting a resource (turn editing off to verify this action has been completed)
      20. Try moving a section around (turn editing off to verify this action has been completed successfully)
      21. Try setting a setting a section as the current topic and verify it has been marked visually (turn editing off to verify this action has been completed successfully)
      22. Try editing a section name and verify the section name appears on the main page
      23. Try increasing adding a section at the bottom of the course page when editting is on and verify the section is added

      Weeks:

      1. Set course format to topics
      2. Disable course ajax in Admin > Appearance > Ajax and Javascript
      3. Verify that the course appears as before change visually
      4. Turn editting on/off with the button in top right and verify you are returned to where you were
      5. Turn editting on/off with the button in the settings menu and verify you are returned to where you were
      6. Try adding a resource to a section (and you are returned to where you were)
      7. Try moving a resource between sections (and you are returned to where you were)
      8. Try indenting a section (and you are returned to where you were)
      9. Try editing a resource (and you are returned to where you were)
      10. Try hiding a resource (and you are returned to where you were)
      11. Try deleting a resource (and you are returned to where you were)
        # Try duplicating a resource (and you are returned to where you were)
      12. Try moving a section
      13. Try hiding a section
      14. Verify that the current week is highlighted
      15. Turn AJAX back on
      16. Try moving a resource between sections (turn editing off to verify this action has been completed)
      17. Try indenting a section (turn editing off to verify this action has been completed)
      18. Try hiding a resource (turn editing off to verify this action has been completed)
      19. Try deleting a resource (turn editing off to verify this action has been completed)
      20. Try moving a section around (turn editing off to verify this action has been completed successfully)
      21. Try editing a section name and verify the section name appears on the main page
      22. Try increasing the number of sections in the course settings and verify the section is added
      23. Try increasing adding a section at the bottom of the course page when editting is on and verify the section is added

      Test multiple page

      Setup:

      1. Create a course with course display setting set to 'multiple page'

      Topics:

      1. Set course format to topics
      2. Disable course ajax in Admin > Appearance > Ajax and Javascript
      3. Verify that the main course page lists a summary of sections
      4. Visit each section and navigate between them with the next/previous links
      5. Attempt to navigate to past the last sections and before the first section with the next/previous links
      6. Verify that the navigation links work correctly to go to search section

      For the following tests, verify on both section page and index page

      1. Turn editting on/off with the button in top right and verify you are returned to where you were
      2. Turn editting on/off with the button in the settings menu and verify you are returned to where you were
      3. Try adding a resource to a section (and you are returned to where you were)
      4. Try moving a resource between sections (and you are returned to where you were)
      5. Try indenting a section (and you are returned to where you were)
      6. Try editing a resource (and you are returned to where you were)
      7. Try hiding a resource (and you are returned to where you were)
      8. Try deleting a resource (and you are returned to where you were)
        # Try duplicating a resource (and you are returned to where you were)
      9. Try hiding a section
      10. Try setting a setting a section as the current topic and verify it has been marked visually

      Verify that you can do these actions in the 'index page' but not on single section page:

      1. Try moving a section.
      2. Turn AJAX back on

      For the following tests, verify on both section page and index page

      1. Try moving a resource between sections (the general section on section page) (turn editing off to verify this action has been completed)
      2. Try indenting a section (turn editing off to verify this action has been completed)
      3. Try editing a resource (turn editing off to verify this action has been completed)
      4. Try hiding a resource (turn editing off to verify this action has been completed)
      5. Try deleting a resource (turn editing off to verify this action has been completed)
        # Try duplicating a resource (turn editing off to verify this action has been completed)
      6. Try hiding a section (turn editing off to verify this action has been completed)
      7. Try setting a setting a section as the current topic and verify it has been marked visually (turn editing off to verify this action has been completed)

      Weeks:

      1. Set course format to weeks
      2. Disable course ajax in Admin > Appearance > Ajax and Javascript
      3. Verify that the main course page lists a summary of sections
      4. Visit each section and navigate between them with the next/previous links
      5. Attempt to navigate to past the last sections and before the first section with the next/previous links
      6. Verify that the navigation links work correctly to go to search section

      For the following tests, verify on both section page and index page

      1. Turn editting on/off with the button in top right and verify you are returned to where you were
      2. Turn editting on/off with the button in the settings menu and verify you are returned to where you were
      3. Try adding a resource to a section (and you are returned to where you were)
      4. Try moving a resource between sections (and you are returned to where you were)
      5. Try indenting a section (and you are returned to where you were)
      6. Try editing a resource (and you are returned to where you were)
      7. Try hiding a resource (and you are returned to where you were)
      8. Try deleting a resource (and you are returned to where you were)
        # Try duplicating a resource (and you are returned to where you were)
      9. Try hiding a section
      10. Verify the current week is marked visually

      Verify that you can do these actions in the 'index page' but not on single section page:

      1. Try moving a section.
      2. Turn AJAX back on

      For the following tests, verify on both section page and index page

      1. Try moving a resource between sections (the general section on section page) (turn editing off to verify this action has been completed)
      2. Try indenting a section (turn editing off to verify this action has been completed)
      3. Try editing a resource (turn editing off to verify this action has been completed)
      4. Try hiding a resource (turn editing off to verify this action has been completed)
      5. Try deleting a resource (turn editing off to verify this action has been completed)
        # Try duplicating a resource (turn editing off to verify this action has been completed)
      6. Try hiding a section (turn editing off to verify this action has been completed)

      Section Links Block:

      1. Try adding the section links block and verify that the section links are working correctly

      Run phpunit tests

      1. There should be no failures.
      Show
      Test social format Create a course with format set to social Ensure the social format is working as expected Test scorm format Create a course with format set to scorm Upload a packaegthe social format is working as expected Test single page Setup: Create a course with course display setting set to 'single page' Topics: Set course format to topics Disable course ajax in Admin > Appearance > Ajax and Javascript Verify that the course appears as before change visually Turn editting on/off with the button in top right and verify you are returned to where you were Turn editting on/off with the button in the settings menu and verify you are returned to where you were Try adding a resource to a section (and you are returned to where you were) Try moving a resource between sections (and you are returned to where you were) Try indenting a section (and you are returned to where you were) Try editing a resource (and you are returned to where you were) Try hiding a resource (and you are returned to where you were) Try deleting a resource (and you are returned to where you were) # Try duplicating a resource (and you are returned to where you were) Try moving a section Try hiding a section Try setting a setting a section as the current topic and verify it has been marked visually Turn AJAX back on Try moving a resource between sections (turn editing off to verify this action has been completed) Try indenting a section (turn editing off to verify this action has been completed) Try hiding a resource (turn editing off to verify this action has been completed) Try deleting a resource (turn editing off to verify this action has been completed) Try moving a section around (turn editing off to verify this action has been completed successfully) Try setting a setting a section as the current topic and verify it has been marked visually (turn editing off to verify this action has been completed successfully) Try editing a section name and verify the section name appears on the main page Try increasing adding a section at the bottom of the course page when editting is on and verify the section is added Weeks: Set course format to topics Disable course ajax in Admin > Appearance > Ajax and Javascript Verify that the course appears as before change visually Turn editting on/off with the button in top right and verify you are returned to where you were Turn editting on/off with the button in the settings menu and verify you are returned to where you were Try adding a resource to a section (and you are returned to where you were) Try moving a resource between sections (and you are returned to where you were) Try indenting a section (and you are returned to where you were) Try editing a resource (and you are returned to where you were) Try hiding a resource (and you are returned to where you were) Try deleting a resource (and you are returned to where you were) # Try duplicating a resource (and you are returned to where you were) Try moving a section Try hiding a section Verify that the current week is highlighted Turn AJAX back on Try moving a resource between sections (turn editing off to verify this action has been completed) Try indenting a section (turn editing off to verify this action has been completed) Try hiding a resource (turn editing off to verify this action has been completed) Try deleting a resource (turn editing off to verify this action has been completed) Try moving a section around (turn editing off to verify this action has been completed successfully) Try editing a section name and verify the section name appears on the main page Try increasing the number of sections in the course settings and verify the section is added Try increasing adding a section at the bottom of the course page when editting is on and verify the section is added Test multiple page Setup: Create a course with course display setting set to 'multiple page' Topics: Set course format to topics Disable course ajax in Admin > Appearance > Ajax and Javascript Verify that the main course page lists a summary of sections Visit each section and navigate between them with the next/previous links Attempt to navigate to past the last sections and before the first section with the next/previous links Verify that the navigation links work correctly to go to search section For the following tests, verify on both section page and index page Turn editting on/off with the button in top right and verify you are returned to where you were Turn editting on/off with the button in the settings menu and verify you are returned to where you were Try adding a resource to a section (and you are returned to where you were) Try moving a resource between sections (and you are returned to where you were) Try indenting a section (and you are returned to where you were) Try editing a resource (and you are returned to where you were) Try hiding a resource (and you are returned to where you were) Try deleting a resource (and you are returned to where you were) # Try duplicating a resource (and you are returned to where you were) Try hiding a section Try setting a setting a section as the current topic and verify it has been marked visually Verify that you can do these actions in the 'index page' but not on single section page: Try moving a section. Turn AJAX back on For the following tests, verify on both section page and index page Try moving a resource between sections (the general section on section page) (turn editing off to verify this action has been completed) Try indenting a section (turn editing off to verify this action has been completed) Try editing a resource (turn editing off to verify this action has been completed) Try hiding a resource (turn editing off to verify this action has been completed) Try deleting a resource (turn editing off to verify this action has been completed) # Try duplicating a resource (turn editing off to verify this action has been completed) Try hiding a section (turn editing off to verify this action has been completed) Try setting a setting a section as the current topic and verify it has been marked visually (turn editing off to verify this action has been completed) Weeks: Set course format to weeks Disable course ajax in Admin > Appearance > Ajax and Javascript Verify that the main course page lists a summary of sections Visit each section and navigate between them with the next/previous links Attempt to navigate to past the last sections and before the first section with the next/previous links Verify that the navigation links work correctly to go to search section For the following tests, verify on both section page and index page Turn editting on/off with the button in top right and verify you are returned to where you were Turn editting on/off with the button in the settings menu and verify you are returned to where you were Try adding a resource to a section (and you are returned to where you were) Try moving a resource between sections (and you are returned to where you were) Try indenting a section (and you are returned to where you were) Try editing a resource (and you are returned to where you were) Try hiding a resource (and you are returned to where you were) Try deleting a resource (and you are returned to where you were) # Try duplicating a resource (and you are returned to where you were) Try hiding a section Verify the current week is marked visually Verify that you can do these actions in the 'index page' but not on single section page: Try moving a section. Turn AJAX back on For the following tests, verify on both section page and index page Try moving a resource between sections (the general section on section page) (turn editing off to verify this action has been completed) Try indenting a section (turn editing off to verify this action has been completed) Try editing a resource (turn editing off to verify this action has been completed) Try hiding a resource (turn editing off to verify this action has been completed) Try deleting a resource (turn editing off to verify this action has been completed) # Try duplicating a resource (turn editing off to verify this action has been completed) Try hiding a section (turn editing off to verify this action has been completed) Section Links Block: Try adding the section links block and verify that the section links are working correctly Run phpunit tests There should be no failures.
    • Affected Branches:
      MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull Master Branch:
      MDL-32508-wip
    • Rank:
      39400

      Description

      This describes the bare minimum we can do to improve things in 2.3 without refactoring course formats (as there isn't time).

      It describes how to improve the weekly and topics formats once the course setting in MDL-32504 is implemented.

      Main course page

      The main course main page should start with the General section (section 0) IFF it contains anything. If it is empty, don't show it.

      Next, show a nicely-formatted list of sections below it with the current one highlighted. For each topic/week there is:

      • the section name (as a link to the separate page, i.e. course/view.php?id=12&topic=3 or course/view.php?id=2&week=3 or course/view.php?id=2&section=3 are all synonyms)
      • the section summary (truncated if it's too long)
      • a completion summary (if applicable, 1/3, 30%, ?)
      • controls to hide/show that section, as usual
      • Ajax drag controls (or arrows for non-JS) to change the order by dragging

      The "zoom" boxes that control 'Show one/all weeks/topics' are no longer needed and should be removed.

      More sections can be added by a big "plus" icon at the bottom, and old ones can be deleted if they are empty.

      Section pages

      Section pages use the same course blocks as the main page.

      The navigation bar at the top should show the section name as a sub page and be selected. It's thus very easy to "go back" one level to the main course page.

      There should also be nav buttons to prev/next sections in the top and bottom of the page.

      The General section (section 0) can be displayed IFF it contains anything. If it is empty, don't show it.

      The section is displayed normally, same as 2.2.

      We need a new interface for copying activities to/from this page. I like this: add select checkboxes to select activities, and then two buttons: [Copy activities] (enabled only when things are checked) and [Paste activities] (enabled only when things have been copied). A small clipboard in user preferences would keep track of those activities that are being copied (per course).

      In 2.3 this would only work within a course, but the same interface would work later across courses.

      Navigation links

      If in paged mode, then the section links are exactly as they are now: course/view.php?id=12&topic=3

      If in scrolling mode (all sections on one page) then the links should be course/view.php?id=12#topic-3

      Check for regressions in modules caused by course assumptions

      For example, make sure that "return to course" actually returns to the appropriate section. Forum module does something like that when you add a new post from the Latest News block. Also editing module settings has a "Save and return to course" button. etc.

        Issue Links

          Activity

          Hide
          Dan Poltawski added a comment -

          the section name (as a link to the separate page, i.e. course/view.php?id=12&topic=3 or course/view.php?id=2&week=3 or course/view.php?id=2&section=3 are all synonyms)

          I think this is going to be a challenging part of this. At the moment the two course formats (topics/weeks) introduce their own param (topic vs week vs section). This is kind of ugly, and is going to be more problematic now we don't have 'sticky' sections and need to pass the param around in urls throughout modules. Perhaps we should standardise it.

          Show
          Dan Poltawski added a comment - the section name (as a link to the separate page, i.e. course/view.php?id=12&topic=3 or course/view.php?id=2&week=3 or course/view.php?id=2&section=3 are all synonyms) I think this is going to be a challenging part of this. At the moment the two course formats (topics/weeks) introduce their own param (topic vs week vs section). This is kind of ugly, and is going to be more problematic now we don't have 'sticky' sections and need to pass the param around in urls throughout modules. Perhaps we should standardise it.
          Hide
          Itamar Tzadok added a comment -

          I don't really mind any changes to the standard topics/weeks formats. So just for the sake of those who do use them, I'm not entirely sure that the "zoom" boxes or at least the "zoom in" boxes are not needed. Suppose a user wants to see all sections on one page, scroll through and zoom into one of them. A "zoom in" button in the section would be useful. Or is there another way to achieve the same effect? Zoom out, however, is probably not needed in the section since it would be in this or that navbar.

          Show
          Itamar Tzadok added a comment - I don't really mind any changes to the standard topics/weeks formats. So just for the sake of those who do use them, I'm not entirely sure that the "zoom" boxes or at least the "zoom in" boxes are not needed. Suppose a user wants to see all sections on one page, scroll through and zoom into one of them. A "zoom in" button in the section would be useful. Or is there another way to achieve the same effect? Zoom out, however, is probably not needed in the section since it would be in this or that navbar.
          Hide
          Martin Dougiamas added a comment -

          We'll need to support the old ones anyway for backward compatibility with all the old links ...

          If you need to just promote one anywhere though (like in nav) I guess we should use "section". Or 'page'. lol

          Actually is "page" such a bad idea ...?

          Show
          Martin Dougiamas added a comment - We'll need to support the old ones anyway for backward compatibility with all the old links ... If you need to just promote one anywhere though (like in nav) I guess we should use "section". Or 'page'. lol Actually is "page" such a bad idea ...?
          Hide
          Martin Dougiamas added a comment -

          Itamar, over the years there's been much confusion and complaints about the user zoom boxes we have had since Moodle 1.x . eg teacher's don't know if it's changing for all the class or just them, and students wondering where the course went, etc.

          If we put everything on one page we're back to the scroll of death.

          The proposal above has links to the pages from the index page (when in paged layout) so it's basically a "zoom" if you like.

          But we can definitely revisit user preferences in 2.4 and other formats.

          Show
          Martin Dougiamas added a comment - Itamar, over the years there's been much confusion and complaints about the user zoom boxes we have had since Moodle 1.x . eg teacher's don't know if it's changing for all the class or just them, and students wondering where the course went, etc. If we put everything on one page we're back to the scroll of death. The proposal above has links to the pages from the index page (when in paged layout) so it's basically a "zoom" if you like. But we can definitely revisit user preferences in 2.4 and other formats.
          Hide
          Itamar Tzadok added a comment -

          As I said as long as the format controls the display and navigation, I don't mind any of these changes. It is just something that I find useful while developing or reviewing course content, that is, show all sections to get the big picture or print the course content, and zoom in on one which looks particularly interesting or needs further development. If it's confusing in browse mod, you can remove it there but keep it in editing mod with the other editing controls, for the teacher to use when needed.

          Show
          Itamar Tzadok added a comment - As I said as long as the format controls the display and navigation, I don't mind any of these changes. It is just something that I find useful while developing or reviewing course content, that is, show all sections to get the big picture or print the course content, and zoom in on one which looks particularly interesting or needs further development. If it's confusing in browse mod, you can remove it there but keep it in editing mod with the other editing controls, for the teacher to use when needed.
          Hide
          Dan Poltawski added a comment - - edited

          I just grepped around for any uses of 'page' parameter that might clash, couldn't find any so I think that is good. Only annoyance is of course $PAGE collision..

          Show
          Dan Poltawski added a comment - - edited I just grepped around for any uses of 'page' parameter that might clash, couldn't find any so I think that is good. Only annoyance is of course $PAGE collision..
          Hide
          Martin Dougiamas added a comment -

          Yes, that's a pretty major annoyance. I think I'm tipping towards $section on balance, even considering the human readability angle. It's just clearer and "correct".

          Show
          Martin Dougiamas added a comment - Yes, that's a pretty major annoyance. I think I'm tipping towards $section on balance, even considering the human readability angle. It's just clearer and "correct".
          Hide
          Dan Poltawski added a comment -

          Section is currently used for some actions on the course page (which isn't a dealbraker, but it does mean have to be careful working out what is currently calling the page with section set)

          Show
          Dan Poltawski added a comment - Section is currently used for some actions on the course page (which isn't a dealbraker, but it does mean have to be careful working out what is currently calling the page with section set)
          Hide
          Martin Dougiamas added a comment -

          Let's meet tomorrow morning to finalise the things to do here.

          Show
          Martin Dougiamas added a comment - Let's meet tomorrow morning to finalise the things to do here.
          Hide
          Dan Poltawski added a comment -

          So i've reached the tricky part of this..

          'Check for regressions in modules caused by course assumptions'. It would be fine if we ALWAYS editting on either the 'main page' OR the 'section page' (i.e. if coursedisplay was switched on), but because its possible to do editting on both pages we need to pass around a 'do we return to section page' flag to all sorts of places in order to stop users being returned to somewhere alien.

          Show
          Dan Poltawski added a comment - So i've reached the tricky part of this.. 'Check for regressions in modules caused by course assumptions'. It would be fine if we ALWAYS editting on either the 'main page' OR the 'section page' (i.e. if coursedisplay was switched on), but because its possible to do editting on both pages we need to pass around a 'do we return to section page' flag to all sorts of places in order to stop users being returned to somewhere alien.
          Hide
          Dan Poltawski added a comment -

          The good news is that ajax editing at least significantly reduces the impact of this.

          Show
          Dan Poltawski added a comment - The good news is that ajax editing at least significantly reduces the impact of this.
          Hide
          Dan Poltawski added a comment -

          My branch now has a (quite hastily done in case of weeks) implemtnation of this for both topics and weeks. Unfortunately i'm lossing confidence in this the more I do!

          Show
          Dan Poltawski added a comment - My branch now has a (quite hastily done in case of weeks) implemtnation of this for both topics and weeks. Unfortunately i'm lossing confidence in this the more I do!
          Hide
          Dan Poltawski added a comment -

          Ok, think i've done all I can on this for this weekend so flicking the switch.

          Will write up more about this tomorrow.

          Show
          Dan Poltawski added a comment - Ok, think i've done all I can on this for this weekend so flicking the switch. Will write up more about this tomorrow.
          Hide
          Dan Poltawski added a comment -

          Two notes before I go..

          1) this needs lots of people to look at it!
          2) It needs prettifying, I tried to make it look sensible, but then lost a lot of time trying to tweak tiny little things so have mostly let that be for now

          Show
          Dan Poltawski added a comment - Two notes before I go.. 1) this needs lots of people to look at it! 2) It needs prettifying, I tried to make it look sensible, but then lost a lot of time trying to tweak tiny little things so have mostly let that be for now
          Hide
          Dan Poltawski added a comment -

          So to explain a bit more about this..

          I've included MDL-32504 (new coursedisplay setting) and MDL-32505 (drop course_display table) in this pull request because I don't think it makes sense without all 3 being integrated.

          In terms of this change i've tried to do what I can with a limited amount of time! There are going to be problems that will need to be ironed out, i've no doubt about it.

          1. Formats have been standardised to use a 'section' url param rather than a different one per format. I have added some (horrible because it redirects after output) backwards compatibility with the old params (e.g. week). The only places this should be linked from is peoples own content (and it would previously break if you altered the course format setting)
          2. Introduced a new course_get_url function for returning the canonical url to a course (section), this will observe the new coursedisplay setting and link to a specific section if in that mode. Places returning to a section have been converted to use this
          3. I deprecated callback_format_get_section_url since formats should now all use this standardised param
          4. Places which go back to a section and (e.g. editsection) had a parameters added to determine if they return to a section or the course index page. This is because you can edit from either the index page or the section page so need to be returned to where you came from
          5. Topics and weeks sections were converted to use a renderer. Note that this renderer isn't a very good one because it doesn't really observe a very good 'view' (as in MVC) separation. Also it had to echo output due to print_section(). There is a lot of unraveling we can do to make this better designed in future relases.
          6. The topics and weeks changes are clearly the parts which are going to have biggest impact and there is for sure some design work to be done to make this prettier
          Show
          Dan Poltawski added a comment - So to explain a bit more about this.. I've included MDL-32504 (new coursedisplay setting) and MDL-32505 (drop course_display table) in this pull request because I don't think it makes sense without all 3 being integrated. In terms of this change i've tried to do what I can with a limited amount of time! There are going to be problems that will need to be ironed out, i've no doubt about it. Formats have been standardised to use a 'section' url param rather than a different one per format. I have added some (horrible because it redirects after output) backwards compatibility with the old params (e.g. week). The only places this should be linked from is peoples own content (and it would previously break if you altered the course format setting) Introduced a new course_get_url function for returning the canonical url to a course (section), this will observe the new coursedisplay setting and link to a specific section if in that mode. Places returning to a section have been converted to use this I deprecated callback_format_get_section_url since formats should now all use this standardised param Places which go back to a section and (e.g. editsection) had a parameters added to determine if they return to a section or the course index page. This is because you can edit from either the index page or the section page so need to be returned to where you came from Topics and weeks sections were converted to use a renderer. Note that this renderer isn't a very good one because it doesn't really observe a very good 'view' (as in MVC) separation. Also it had to echo output due to print_section(). There is a lot of unraveling we can do to make this better designed in future relases. The topics and weeks changes are clearly the parts which are going to have biggest impact and there is for sure some design work to be done to make this prettier
          Hide
          Martin Dougiamas added a comment -

          I'm looking at the demo site and have some GUI feedback:

          http://moodle.danpoltawski.co.uk/courseformats/course/view.php?id=2&section=2

          1) The title of the current section should be at the top of the page, in the middle between the navigation links left and right. This looks to be purely a CSS issue for the H2 there.

          2) I think it would be good to add "Return to main course page" as a link between the left/right links at the bottom of a section page. That gives people FOUR ways to get back.

          3) The navigation bar is missing the last element that shows the current page.

          Home > Courses > course > 7 May - 13 May

          This will also fix the navigation block menu, which is not showing all the sections as it should, and should show the current section expanded.

          4) Pressing "Turn editing on" on the section page bounces me to the main page. It should stay in that section page until they manually go back there via navigation links mentioned above.

          5) I just realised the General section is a great way to move activities from one section page to another, even without using the main page! Bonus!

          6) I think (85% sure) the general section should be shown at the very top of the page, above the navigation links.

          7) The "section move" arrows should be removed in the section page.

          On http://moodle.danpoltawski.co.uk/courseformats/course/view.php?id=2

          8) In Weekly format, the section numbers are not required.

          9) You can get rid of the H2 "Weekly outline" and "Topic outline" from the top of the course main page. A lot of people have complained about them in the past. Their only use MIGHT be to someone using a screen reader (that's doubtful too I guess) so perhaps to be safe we should keep the H2 and just style them as accesshide.

          10) I see "Add an additional section" and it works, but I can't delete any sections.

          Show
          Martin Dougiamas added a comment - I'm looking at the demo site and have some GUI feedback: http://moodle.danpoltawski.co.uk/courseformats/course/view.php?id=2&section=2 1) The title of the current section should be at the top of the page, in the middle between the navigation links left and right. This looks to be purely a CSS issue for the H2 there. 2) I think it would be good to add "Return to main course page" as a link between the left/right links at the bottom of a section page. That gives people FOUR ways to get back. 3) The navigation bar is missing the last element that shows the current page. Home > Courses > course > 7 May - 13 May This will also fix the navigation block menu, which is not showing all the sections as it should, and should show the current section expanded. 4) Pressing "Turn editing on" on the section page bounces me to the main page. It should stay in that section page until they manually go back there via navigation links mentioned above. 5) I just realised the General section is a great way to move activities from one section page to another, even without using the main page! Bonus! 6) I think (85% sure) the general section should be shown at the very top of the page, above the navigation links. 7) The "section move" arrows should be removed in the section page. On http://moodle.danpoltawski.co.uk/courseformats/course/view.php?id=2 8) In Weekly format, the section numbers are not required. 9) You can get rid of the H2 "Weekly outline" and "Topic outline" from the top of the course main page. A lot of people have complained about them in the past. Their only use MIGHT be to someone using a screen reader (that's doubtful too I guess) so perhaps to be safe we should keep the H2 and just style them as accesshide. 10) I see "Add an additional section" and it works, but I can't delete any sections.
          Hide
          Dan Poltawski added a comment -

          1) Not done yet. This is a bit weird because of the course completion help button
          2) Note done because want to think about 1)
          3) I've made sections always display in navigation. Think this might be contraversial, but MD thinks its best
          4) Fixed
          5) Bonus!
          6) Done
          7) This is an AJAX issue only, will create another issue for Andrew & Ruslan
          8) Fixed
          9) Done
          10) Need to think about how this works

          Show
          Dan Poltawski added a comment - 1) Not done yet. This is a bit weird because of the course completion help button 2) Note done because want to think about 1) 3) I've made sections always display in navigation. Think this might be contraversial, but MD thinks its best 4) Fixed 5) Bonus! 6) Done 7) This is an AJAX issue only, will create another issue for Andrew & Ruslan 8) Fixed 9) Done 10) Need to think about how this works
          Hide
          Itamar Tzadok added a comment -

          Could you please add API for a format to set the PAGE before any output?

          Consider that it may make more modular sense if the course (/course/view.php) just prepares general settings and objects, and all output is done by the format so that the format could actually fulfill its purpose and format the display head to toe.

          The simplest API before any 2.4 massive refactoring would probably be to include a designated format file if exists right before the OUTPUT->header, say preformat.php, which could be implemented in format plugins and used for PAGE adjustments.

          Show
          Itamar Tzadok added a comment - Could you please add API for a format to set the PAGE before any output? Consider that it may make more modular sense if the course (/course/view.php) just prepares general settings and objects, and all output is done by the format so that the format could actually fulfill its purpose and format the display head to toe. The simplest API before any 2.4 massive refactoring would probably be to include a designated format file if exists right before the OUTPUT->header, say preformat.php, which could be implemented in format plugins and used for PAGE adjustments.
          Hide
          Dan Poltawski added a comment -

          Hi Itamar,

          That was what I was hoping to do, but it turns out to be too much work. For example we really need to fix up print_section and get_all_mods to make this work sanely.

          At the moment the API works exactly the same as 2.2 (as in your have a file included and you can choose to take note of the variables which are in scope..) with the addition of a new parameter and setting that course formats can choose to support.

          Show
          Dan Poltawski added a comment - Hi Itamar, That was what I was hoping to do, but it turns out to be too much work. For example we really need to fix up print_section and get_all_mods to make this work sanely. At the moment the API works exactly the same as 2.2 (as in your have a file included and you can choose to take note of the variables which are in scope..) with the addition of a new parameter and setting that course formats can choose to support.
          Hide
          Martin Dougiamas added a comment -

          Yes, better to keep all API changes to one release (2.4) to help authors and users of popular course formats.

          Show
          Martin Dougiamas added a comment - Yes, better to keep all API changes to one release (2.4) to help authors and users of popular course formats.
          Hide
          Itamar Tzadok added a comment -

          Just wanted to note that including before output a preformat.php if it exists in the format plugin shouldn't change anything in what authors and users of existing formats are doing but could help format plugins such as my own get designated navigation and other elements right, and I'm just a bit concerned that 2.4 may take a while. Nonetheless thanks for considering.

          Show
          Itamar Tzadok added a comment - Just wanted to note that including before output a preformat.php if it exists in the format plugin shouldn't change anything in what authors and users of existing formats are doing but could help format plugins such as my own get designated navigation and other elements right, and I'm just a bit concerned that 2.4 may take a while. Nonetheless thanks for considering.
          Hide
          Sam Hemelryk added a comment -

          Hi Dan,

          The code is looking really excellent, thats a large change set and I only spotted a couple of very minor things that require fixing.

          • FIXME: danp in the tests still. I saw your question to Petr so just looks like a replacement table needs to be used.
          • Typo in lib/navigationlib comment: Depricated. That debugging line should probably use DEBUG_DEVELOPER given that it is related to a need to upgrade code.
          • You added an empty line to the end of course/format/topic/styles.css.
          • course/format/README.txt needs to be updated. Also perhaps worth adding course/format/upgrade.txt now.
          • backup and restore needs to be fixed up, but that should be easy as pie.

          There was one thing that caught my eye, not something necessaryily worth changing, but still something that confused me to begin with and worth discussing and that is format_renderer_base.
          The first think that struck me about this was awesome you've found smart way to abstract course format renderers making parts of them generic. However upon looking into it I see that extending renderers are only actually being used within the format, and this is just abstracting code to reduce the amount of duplicated code within course format renderers.
          I suppose I had assumed that the renderer was something every format would and should be using, but it really something that you can optionally use if you want to produce a format that looks like topics, or weeks.
          None of that is actually bad by the way, I'm just retelling my initial experience. In fact the comment in the constructor provide a little bit of a hint.
          I wonder whether it would be clearer to people looking into course formats if that class were renamed to something like format_section_renderer_base, and a comment added noting that this class is provided for convenience and doesn't have to be abstracted in a course format.
          The other thing that I noted was that while all methods of the renderers were public in fact only print_multiple_section_page and print_single_section_page are actually used publicly. Given that course formats are still due to be looked at in 2.4 (am I correct in thinking that actually) perhaps it would be worth making the other methods protocted?
          Again its not at all required to do so, and I'm not convinced it should be done, its just an idea. Making them protected would make it very obvious to those developing formats how to use that class (albeit copy and paste will to) and perhaps there is some benefit in having a smaller public API introduced in 2.3 given it will likely change again in 2.4. I don't really know a curious thought that struck me is all.

          Finally a couple of ideas that I occured to me while reading through this code:

          • In a funny way there is a loss of functionality introduced by deprecating the callback_courseformat_get_section_url method. That method appears to let a format add not just a section param but in fact set any URL they wish, and with any params they wanted. While I know it was never used like that in core I've no clue whether it was ever used like that in practice. I could imagine how that would be useful however. Either way not really a problem, its deprecated and things will still work so anyone who has made use of it in such a way would have the opertunity to find another way to achieve any additional functionality they had wrapped into it.
          • I think it that creating a defaultcoursedisplay setting for the site would be a smart idea. A bit like default course format setting. Of course that could be done as a separate issue, or perhaps given the changes in 2.4 would only introduce clutter.

          Other than that it everything looks spot on, and appears to function perfectly.

          I've reopened this now.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi Dan, The code is looking really excellent, thats a large change set and I only spotted a couple of very minor things that require fixing. FIXME: danp in the tests still. I saw your question to Petr so just looks like a replacement table needs to be used. Typo in lib/navigationlib comment: Depricated. That debugging line should probably use DEBUG_DEVELOPER given that it is related to a need to upgrade code. You added an empty line to the end of course/format/topic/styles.css. course/format/README.txt needs to be updated. Also perhaps worth adding course/format/upgrade.txt now. backup and restore needs to be fixed up, but that should be easy as pie. There was one thing that caught my eye, not something necessaryily worth changing, but still something that confused me to begin with and worth discussing and that is format_renderer_base. The first think that struck me about this was awesome you've found smart way to abstract course format renderers making parts of them generic. However upon looking into it I see that extending renderers are only actually being used within the format, and this is just abstracting code to reduce the amount of duplicated code within course format renderers. I suppose I had assumed that the renderer was something every format would and should be using, but it really something that you can optionally use if you want to produce a format that looks like topics, or weeks. None of that is actually bad by the way, I'm just retelling my initial experience. In fact the comment in the constructor provide a little bit of a hint. I wonder whether it would be clearer to people looking into course formats if that class were renamed to something like format_section_renderer_base, and a comment added noting that this class is provided for convenience and doesn't have to be abstracted in a course format. The other thing that I noted was that while all methods of the renderers were public in fact only print_multiple_section_page and print_single_section_page are actually used publicly. Given that course formats are still due to be looked at in 2.4 (am I correct in thinking that actually) perhaps it would be worth making the other methods protocted? Again its not at all required to do so, and I'm not convinced it should be done, its just an idea. Making them protected would make it very obvious to those developing formats how to use that class (albeit copy and paste will to) and perhaps there is some benefit in having a smaller public API introduced in 2.3 given it will likely change again in 2.4. I don't really know a curious thought that struck me is all. Finally a couple of ideas that I occured to me while reading through this code: In a funny way there is a loss of functionality introduced by deprecating the callback_courseformat_get_section_url method. That method appears to let a format add not just a section param but in fact set any URL they wish, and with any params they wanted. While I know it was never used like that in core I've no clue whether it was ever used like that in practice. I could imagine how that would be useful however. Either way not really a problem, its deprecated and things will still work so anyone who has made use of it in such a way would have the opertunity to find another way to achieve any additional functionality they had wrapped into it. I think it that creating a defaultcoursedisplay setting for the site would be a smart idea. A bit like default course format setting. Of course that could be done as a separate issue, or perhaps given the changes in 2.4 would only introduce clutter. Other than that it everything looks spot on, and appears to function perfectly. I've reopened this now. Cheers Sam
          Hide
          Dan Poltawski added a comment -

          Thanks Sam for the review - to summarise some things we just chatted about:

          • I'll fix the issues you spotted - thanks
          • Completely agree about format_renderer_base being too section specific. In fact I meant to comment about this here. It is mostly an evolution of trying to abstract things (quickly!). In fact format_section_renderer_base works fine for me so I will switch to that.
          • Making these methods protected also makes sense, they are public due to the evolution of the code. Making these public is more a 2.4 thing and indeed we don't really want people to start depending on this as a public API.

          Regarding callback_courseformat_get_section_url - I think I did have a look through the contrib plugins to see if anyone was using it and they weren't. Perhaps we shouldn't warn about it at all because its just going to be changed again in 2.4..

          Show
          Dan Poltawski added a comment - Thanks Sam for the review - to summarise some things we just chatted about: I'll fix the issues you spotted - thanks Completely agree about format_renderer_base being too section specific. In fact I meant to comment about this here. It is mostly an evolution of trying to abstract things (quickly!). In fact format_section_renderer_base works fine for me so I will switch to that. Making these methods protected also makes sense, they are public due to the evolution of the code. Making these public is more a 2.4 thing and indeed we don't really want people to start depending on this as a public API. Regarding callback_courseformat_get_section_url - I think I did have a look through the contrib plugins to see if anyone was using it and they weren't. Perhaps we shouldn't warn about it at all because its just going to be changed again in 2.4..
          Hide
          Dan Poltawski added a comment - - edited

          OK i've fixed up all those issues and i've also rebased against integration/master as sam mentioned conflicts.

          I haven't updated: course/format/README.txt (though I have added upgrade.txt). I think that readme file is outdated and not placed properly. I propose we delete it and move the docs to the wiki. If we want to do that can we do it in another issue?

          Show
          Dan Poltawski added a comment - - edited OK i've fixed up all those issues and i've also rebased against integration/master as sam mentioned conflicts. I haven't updated: course/format/README.txt (though I have added upgrade.txt). I think that readme file is outdated and not placed properly. I propose we delete it and move the docs to the wiki. If we want to do that can we do it in another issue?
          Hide
          Andrew Nicols added a comment -

          Hi Dan,

          Whilst having a play I noticed that the add section link div is in the <ul> element instead of after it. This causes issues with drag/drop

          Cheers,
          A

          Show
          Andrew Nicols added a comment - Hi Dan, Whilst having a play I noticed that the add section link div is in the <ul> element instead of after it. This causes issues with drag/drop Cheers, A
          Show
          Andrew Nicols added a comment - One fix for you sir: MDL-32508 -master-1 https://git.luns.net.uk/moodle.git/commitdiff/b3848d37b2c09eefce1209953e5cfcb09b5647c7
          Hide
          Dan Poltawski added a comment -

          Thanks Andrew - good spotting. I've pulled that. Actually if you are having a play and have some time it would be good if you look at the output to see if I messed up anywhere else.

          Show
          Dan Poltawski added a comment - Thanks Andrew - good spotting. I've pulled that. Actually if you are having a play and have some time it would be good if you look at the output to see if I messed up anywhere else.
          Hide
          Andrew Nicols added a comment -

          Having more of a play, with course layout set to show one topic per page, it'd be nice if the navigation links also had a link to the course, as well as the previous and next topics.

          Trying to navigate to an invalid section number causes a Coding error which should probably be caught with an invalid section error instead.

          Show
          Andrew Nicols added a comment - Having more of a play, with course layout set to show one topic per page, it'd be nice if the navigation links also had a link to the course, as well as the previous and next topics. Trying to navigate to an invalid section number causes a Coding error which should probably be caught with an invalid section error instead.
          Show
          Andrew Nicols added a comment - Fix for invalid sectionid: MDL-32508 -master-2 - https://git.luns.net.uk/moodle.git/commitdiff/d4c3efb75581ac8646f900b1c1a6e117e97309ba
          Hide
          Dan Poltawski added a comment -

          it'd be nice if the navigation links also had a link to the course,

          Andrew, i.e. Martins comment?

          2) I think it would be good to add "Return to main course page" as a link between the left/right links at the bottom of a section page. That gives people FOUR ways to get back.

          Show
          Dan Poltawski added a comment - it'd be nice if the navigation links also had a link to the course, Andrew, i.e. Martins comment? 2) I think it would be good to add "Return to main course page" as a link between the left/right links at the bottom of a section page. That gives people FOUR ways to get back.
          Hide
          Andrew Nicols added a comment -

          yup - precisely

          More thoughts:

          • when in display one mode, I think we should still highlight upcoming events for a topic. It could be too easy to miss an assignment because a user didn't open a section for a past topic with relevant work still to do
          Show
          Andrew Nicols added a comment - yup - precisely More thoughts: when in display one mode, I think we should still highlight upcoming events for a topic. It could be too easy to miss an assignment because a user didn't open a section for a past topic with relevant work still to do
          Hide
          Dan Poltawski added a comment -

          I've just rebased again on top of integration since I just integrated a conflicting change.

          Show
          Dan Poltawski added a comment - I've just rebased again on top of integration since I just integrated a conflicting change.
          Hide
          Dan Poltawski added a comment -

          I've created MDL-32765 to capture the remaining for this, post integration.

          Show
          Dan Poltawski added a comment - I've created MDL-32765 to capture the remaining for this, post integration.
          Hide
          Sam Hemelryk added a comment -

          Thanks Dan, has been integrated now, starting the testing.

          Show
          Sam Hemelryk added a comment - Thanks Dan, has been integrated now, starting the testing.
          Hide
          Sam Hemelryk added a comment -

          Attaching screenshot of minor CSS display issue. class=nolist is needed somewhere.

          Show
          Sam Hemelryk added a comment - Attaching screenshot of minor CSS display issue. class=nolist is needed somewhere.
          Hide
          Dan Poltawski added a comment -

          Andrew: thanks for your invalid sectionid fix - i've pulled that in.

          Show
          Dan Poltawski added a comment - Andrew: thanks for your invalid sectionid fix - i've pulled that in.
          Hide
          Sam Hemelryk added a comment -

          Hi Dan,

          Just been testing this now and have finished everything off.
          I noted a few things, all which I think could be fixed as separate issues if you want.

          1. Attached a screenshot about the CSS issue in Firefox, was fine in chrome and didn't test the rest.
          2. With AJAX off, editing on, and viewing a specific section in a course with one section per page I find any action performed on an activity leads me back to the course front page rather than the section. Works fine with AJAX on and just activity actions are affected, section actions work fine.

          Everything else works perfectly.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi Dan, Just been testing this now and have finished everything off. I noted a few things, all which I think could be fixed as separate issues if you want. Attached a screenshot about the CSS issue in Firefox, was fine in chrome and didn't test the rest. With AJAX off, editing on, and viewing a specific section in a course with one section per page I find any action performed on an activity leads me back to the course front page rather than the section. Works fine with AJAX on and just activity actions are affected, section actions work fine. Everything else works perfectly. Cheers Sam
          Hide
          Dan Poltawski added a comment -

          I've created those two issues found in testing as subtasks of MDL-32765

          Show
          Dan Poltawski added a comment - I've created those two issues found in testing as subtasks of MDL-32765
          Hide
          Eloy Lafuente (stronk7) added a comment -
          UPDATE tracker_issues
             SET status = 'Closed',
                comment = 'Thanks!'
          WHEN participants = 'Did a gorgeous work'
          

          This landed upstream some hours ago (some - me - developer fell slept in the sofa yesterday before spamming this).

          Show
          Eloy Lafuente (stronk7) added a comment - UPDATE tracker_issues SET status = 'Closed', comment = 'Thanks!' WHEN participants = 'Did a gorgeous work' This landed upstream some hours ago (some - me - developer fell slept in the sofa yesterday before spamming this).
          Hide
          Tim Barker added a comment -

          Test management sub-task to turn Dan's tests into MDLQA tests.

          Show
          Tim Barker added a comment - Test management sub-task to turn Dan's tests into MDLQA tests.
          Hide
          Mary Cooch added a comment -

          Just housekeeping - removing docs_required label as it's documented here http://docs.moodle.org/23/en/Course_formats

          Show
          Mary Cooch added a comment - Just housekeeping - removing docs_required label as it's documented here http://docs.moodle.org/23/en/Course_formats
          Hide
          David Herney Bernal added a comment -

          I upgraded the onetopic course format to moodle 2.3. However, I am think that current section management is some "closed". Currently I am using the attribute $USER->display[$course_id] to more a fluid user experience in the navigation. I think that current section is more easy to manage in session, not sending the parameter in each request, exist many source pages and is very possible that some page do not send this parameter (session=n) ¿What problem is handle the state of this variable in session?

          (format: http://moodle.org/plugins/view.php?plugin=format_onetopic)

          Show
          David Herney Bernal added a comment - I upgraded the onetopic course format to moodle 2.3. However, I am think that current section management is some "closed". Currently I am using the attribute $USER->display [$course_id] to more a fluid user experience in the navigation. I think that current section is more easy to manage in session, not sending the parameter in each request, exist many source pages and is very possible that some page do not send this parameter (session=n) ¿What problem is handle the state of this variable in session? (format: http://moodle.org/plugins/view.php?plugin=format_onetopic )
          Hide
          Dan Poltawski added a comment -

          Hi David, using sessions is problematic. Just look at this tracker as an example. Try and do two searches in two tabs (you can't, because it uses session). It leads to very confusing results where action in one window affects the output of another.

          Show
          Dan Poltawski added a comment - Hi David, using sessions is problematic. Just look at this tracker as an example. Try and do two searches in two tabs (you can't, because it uses session). It leads to very confusing results where action in one window affects the output of another.
          Hide
          David Herney Bernal added a comment -

          Hi Dan...

          it is true, session variables are confused when the user navigate in the same course in differents browser tabs

          so... and a mixed between parameters and sessions? if exist the "section" parameter use this, if not, use the session variables. For example, it is good when a user browsing different courses and return to a previous course or in other cases when the parameter "section" is not passed.

          Something else, when not is passed the section parameter, now the "zero section" is asigned but in some cases is possible that the developer need to diferentiate between "not section" and "zero section". It is my case, in the onetopic format. In this moment, I am reading directly the section parameter in format.php file with:
          $section = optional_param('section', -1, PARAM_INT);
          but it is "evil" in there site, acording to the new structure. What would you suggest?

          thanks

          Show
          David Herney Bernal added a comment - Hi Dan... it is true, session variables are confused when the user navigate in the same course in differents browser tabs so... and a mixed between parameters and sessions? if exist the "section" parameter use this, if not, use the session variables. For example, it is good when a user browsing different courses and return to a previous course or in other cases when the parameter "section" is not passed. Something else, when not is passed the section parameter, now the "zero section" is asigned but in some cases is possible that the developer need to diferentiate between "not section" and "zero section". It is my case, in the onetopic format. In this moment, I am reading directly the section parameter in format.php file with: $section = optional_param('section', -1, PARAM_INT); but it is "evil" in there site, acording to the new structure. What would you suggest? thanks
          Hide
          Elizabeth Dalton added a comment -

          We just upgraded from 2.2 to 2.4 this week, and within the past two days we've started to hear from faculty wanting their "zoom boxes" back. There seem to be a number of forum threads within "Using Moodle" saying the same thing. Itamar noted the possibility of this reaction back in April 2012. We'd prefer a site-wide setting allowing us to enable the zoom boxes for our users. Is it possible to discuss this? Should I open a new tracker?

          Show
          Elizabeth Dalton added a comment - We just upgraded from 2.2 to 2.4 this week, and within the past two days we've started to hear from faculty wanting their "zoom boxes" back. There seem to be a number of forum threads within "Using Moodle" saying the same thing. Itamar noted the possibility of this reaction back in April 2012. We'd prefer a site-wide setting allowing us to enable the zoom boxes for our users. Is it possible to discuss this? Should I open a new tracker?
          Hide
          Mary Cooch added a comment -

          Elizabeth - I think there is a discussion about it on this tracker issue: MDL-34824

          Show
          Mary Cooch added a comment - Elizabeth - I think there is a discussion about it on this tracker issue: MDL-34824
          Hide
          Elizabeth Dalton added a comment -

          Thanks, Mary. I've added my comment there.

          Show
          Elizabeth Dalton added a comment - Thanks, Mary. I've added my comment there.

            People

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

              Dates

              • Created:
                Updated:
                Resolved: