Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.0
    • Fix Version/s: None
    • Component/s: Navigation
    • Labels:
      None
    • Difficulty:
      Moderate
    • Affected Branches:
      MOODLE_20_STABLE
    • Rank:
      15498

      Activity

      Hide
      Sam Hemelryk added a comment -

      Hi guys,

      Have attached a patch here to modify navigation to fit the structure proposed in http://docs.moodle.org/en/Development:Navigation_2.0_structure.
      At the same time I have greatly tidied up the navigation code and renamed the blocks to navigation and settings.
      I still have more testing to do before I am ready to commit but if anyone finds a minute to check the code for me I'd be most appreciative.

      Cheers
      Sam

      Show
      Sam Hemelryk added a comment - Hi guys, Have attached a patch here to modify navigation to fit the structure proposed in http://docs.moodle.org/en/Development:Navigation_2.0_structure . At the same time I have greatly tidied up the navigation code and renamed the blocks to navigation and settings. I still have more testing to do before I am ready to commit but if anyone finds a minute to check the code for me I'd be most appreciative. Cheers Sam
      Hide
      Sam Hemelryk added a comment -

      Fixed conflicts after Friday's commits and reattached patch

      Show
      Sam Hemelryk added a comment - Fixed conflicts after Friday's commits and reattached patch
      Hide
      Sam Hemelryk added a comment -

      Hmmm I have found an issue with the navigation in regards to the user context.... which was always a little hacky anyway.
      I'll try to work out a nice solution and ask questions here if I get stuck

      Show
      Sam Hemelryk added a comment - Hmmm I have found an issue with the navigation in regards to the user context.... which was always a little hacky anyway. I'll try to work out a nice solution and ask questions here if I get stuck
      Hide
      Sam Hemelryk added a comment -

      Attached a file that fixes a further couple of navigation issues.

      Show
      Sam Hemelryk added a comment - Attached a file that fixes a further couple of navigation issues.
      Hide
      Martin Dougiamas added a comment -

      When viewing a course:

      ( ! ) Fatal error: Call to a member function make_active() on a non-object in /web/head/lib/navigationlib.php on line 838
      Call Stack

      1. Time Memory Function Location
        1 0.0144 137688 {main}

        ( ) ../view.php:0
        2 0.3645 21880660 core_renderer->header( ) ../view.php:199
        3 0.3790 22505076 core_renderer->render_page_layout( ) ../outputrenderers.php:565
        4 0.3794 22569776 include( '/web/head/theme/base/layout/general.php' ) ../outputrenderers.php:607
        5 0.3794 22571400 moodle_page->has_navbar( ) ../general.php:4
        6 0.3795 22572172 navbar->has_items( ) ../pagelib.php:588
        7 0.3798 22576476 global_navigation->initialise( ) ../navigationlib.php:1585

      Show
      Martin Dougiamas added a comment - When viewing a course: ( ! ) Fatal error: Call to a member function make_active() on a non-object in /web/head/lib/navigationlib.php on line 838 Call Stack Time Memory Function Location 1 0.0144 137688 {main} ( ) ../view.php:0 2 0.3645 21880660 core_renderer->header( ) ../view.php:199 3 0.3790 22505076 core_renderer->render_page_layout( ) ../outputrenderers.php:565 4 0.3794 22569776 include( '/web/head/theme/base/layout/general.php' ) ../outputrenderers.php:607 5 0.3794 22571400 moodle_page->has_navbar( ) ../general.php:4 6 0.3795 22572172 navbar->has_items( ) ../pagelib.php:588 7 0.3798 22576476 global_navigation->initialise( ) ../navigationlib.php:1585
      Hide
      Sam Hemelryk added a comment -

      Hi Martin,

      Here goes the latest patch, will comment on it soon but in short resolved the issue you saw, cleaned up several more context issues and did a bit more tiding.
      At this point I am pretty happy with it, only thing left is the simpletests

      Show
      Sam Hemelryk added a comment - Hi Martin, Here goes the latest patch, will comment on it soon but in short resolved the issue you saw, cleaned up several more context issues and did a bit more tiding. At this point I am pretty happy with it, only thing left is the simpletests
      Hide
      Petr Škoda added a comment -

      +1 (done only quick review)

      Show
      Petr Škoda added a comment - +1 (done only quick review)
      Hide
      Sam Hemelryk added a comment -

      Hi guys,

      Here is the latest patch: fixed conflicts from recent updates and have corrected what the navbar was showing when browsing /my/index.php

      Martin once you are happy with this I will commit.

      Cheers
      Sam

      Show
      Sam Hemelryk added a comment - Hi guys, Here is the latest patch: fixed conflicts from recent updates and have corrected what the navbar was showing when browsing /my/index.php Martin once you are happy with this I will commit. Cheers Sam
      Hide
      Sam Hemelryk added a comment -

      Hi guys, I have just commit my changes to this issue now!

      The following are changes that are made by this commit:

      • Navigation structure follows that prescribed in http://docs.moodle.org/en/Development:Navigation_2.0_structure
      • Renamed global_navigation_tree to navigation
      • Renamed settings_navigation_tree block to settings
      • Added a navigation_node_collection class to better manage nodes
      • Reduced the course content generated by default relying more on AJAX or reloads but reducing the number of Database queries and context checks.
      • Several notable navigation API changes as follows:
        • navigation_node::add method now returns the newly added node rather than the key to get the node.
        • renamed navigation_node::find_child to navigation_node::find
        • renamed navigation_node::get_child to navigation_node::get
        • renamed navigation_node::remove to navigation_node::remove
        • removed navigation_node::content method, now implemented in renderers
        • navigation_node now stores a reference to its parent (if it has one) so iterative routines arn't as frequently needed and a more precise path can be taken to traverse the tree down.
        • removed the following methods from navigation_node as they were no longer being used:
          • add_to_path - No longer needed due to add method refactoring
          • collapse_at_depth - No longer needed
          • collapse_children - No longer needed
          • find_child_depth - No longer needed due to parent reference
          • find_children_by_type - No longer needed
          • get_children_by_type - No longer needed
          • reiterate_active_nodes - No longer needed due to parent reference
          • respect_forced_open - No longer needed due to parent reference
        • several protected methods removed from global_navigation class
      • Generally tidied the code in navigationlib.php

      Please let me know if you spot any breakages.

      Cheers
      Sam

      Show
      Sam Hemelryk added a comment - Hi guys, I have just commit my changes to this issue now! The following are changes that are made by this commit: Navigation structure follows that prescribed in http://docs.moodle.org/en/Development:Navigation_2.0_structure Renamed global_navigation_tree to navigation Renamed settings_navigation_tree block to settings Added a navigation_node_collection class to better manage nodes Reduced the course content generated by default relying more on AJAX or reloads but reducing the number of Database queries and context checks. Several notable navigation API changes as follows: navigation_node::add method now returns the newly added node rather than the key to get the node. renamed navigation_node::find_child to navigation_node::find renamed navigation_node::get_child to navigation_node::get renamed navigation_node::remove to navigation_node::remove removed navigation_node::content method, now implemented in renderers navigation_node now stores a reference to its parent (if it has one) so iterative routines arn't as frequently needed and a more precise path can be taken to traverse the tree down. removed the following methods from navigation_node as they were no longer being used: add_to_path - No longer needed due to add method refactoring collapse_at_depth - No longer needed collapse_children - No longer needed find_child_depth - No longer needed due to parent reference find_children_by_type - No longer needed get_children_by_type - No longer needed reiterate_active_nodes - No longer needed due to parent reference respect_forced_open - No longer needed due to parent reference several protected methods removed from global_navigation class Generally tidied the code in navigationlib.php Please let me know if you spot any breakages. Cheers Sam
      Hide
      Martin Dougiamas added a comment -

      Well done man, looks a lot cleaner now.

      Show
      Martin Dougiamas added a comment - Well done man, looks a lot cleaner now.
      Hide
      Sam Hemelryk added a comment -

      Thanks Martin

      Reopen if anything major arises.

      Show
      Sam Hemelryk added a comment - Thanks Martin Reopen if anything major arises.

        People

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

          Dates

          • Created:
            Updated:
            Resolved: