Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-32709 Feature request: move the Book module from contrib to the Moodle core
  3. MDL-33121

Review notes from book module integration review and make fixes as required

XMLWordPrintable

    • Icon: Sub-task Sub-task
    • Resolution: Fixed
    • Icon: Minor Minor
    • 2.3
    • 2.3
    • Book
    • MOODLE_23_STABLE
    • MOODLE_23_STABLE
    • Hide

      There's no visible changes on the front-end.

      #. add book resource
      #. create some chapters
      #. test the functionality of the book module and make sure nothing is broken.

      Show
      There's no visible changes on the front-end. #. add book resource #. create some chapters #. test the functionality of the book module and make sure nothing is broken.

      During my integration review I noted several things about the new book module which require thought/fixing/clarification.
      As none of them were blockers it was integrated however we should still have a look over this list and act upon each point accordingly:

      Requiring fixes/changes:

      • README.md needs updating to reflect its integration into core. Not urgent but before release probably (defintely "no more developement planned" ).
      • The opening line of the boilerplate for each file is inconsistent. Some say part of Moodle, others Book module for Moodle, and others again Book plugin for Moodle.
      • $PAGE->set_title calls format_string internally (aweful) but mean no need to wrap things in format_string when setting page title[several files affected].
      • $PAGE->set_heading is the same as above.
      • booktool_***_extend_settings_navigation no need to check modname !== book that has happened within the calling function book_extend_settings_navigation. (Lots of unused globals in those functions as well).
      • book_preload_chapters: next and prev values are never set correctly. Not a bug as they are also not used presently (but should be fixed or removed none the less).
      • tool/exportimscp/index.php unused lang string vars.

      Requiring clarification:

      • I couldn't find or determine a use for book_log function in locallib.php
      • PARAM_RAW for the chapter title, is that correct will we allow HTML + friends there?
      • Is there a need to add mod_book as a body class. You can be sure the body class path-mod-book has been added for all pages within the book module.
      • I couldn't find the use of the chapterscount string. If it is used should that have a {$a}.

      Improvements, usability, accessibility, ideas:

      • It would be nice if when editing the first chapter (such as when you've just created the activity) if the would freeze the "subchapter" option and add a note about it being frozen because the first page cannot be a subchapter. Just a smidge better than ignoring it for friendliness.
      • book_get_toc would be great to convert output to use either a renderer preferably or at least standard output mechanics.
      • Simple observation renderers arn't being used, it would be great if we could convert things to make use of a renderer, simple as it may be.

            rwijaya Rossiani Wijaya
            samhemelryk Sam Hemelryk
            Sam Hemelryk Sam Hemelryk
            Dan Poltawski Dan Poltawski
            Tim Barker Tim Barker
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

              Created:
              Updated:
              Resolved:

                Error rendering 'clockify-timesheets-time-tracking-reports:timer-sidebar'. Please contact your Jira administrators.