Details

    • Testing Instructions:
      Hide
      1. Create a new page resource
      2. Confirm that the form is usable
      3. Confirm that all the fields are correctly saved
      4. Confirm that the default settings set by an admin are used when create a new page (My home ► Site administration ► Plugins ► Activity modules ► Page)
      Show
      Create a new page resource Confirm that the form is usable Confirm that all the fields are correctly saved Confirm that the default settings set by an admin are used when create a new page (My home ► Site administration ► Plugins ► Activity modules ► Page)
    • Affected Branches:
      MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-38611-master-int
    • Rank:
      48638

      Description

      For now, I can only see that the description is far too important than the page content. Also the "Options" section can probably be part of another without ruining the user experience.

        Issue Links

          Activity

          Hide
          Helen Foster added a comment -

          MDL-38614 will definitely help the page settings look a lot better!

          Please note that 'Options' has been renamed 'Display options' in en_fix. I think it's OK having the 2 settings in the section 'Display options'; it keeps General short and consistent with other resources, also the setting 'Display page description' wouldn't look nice next to 'Display description on course page'!

          Show
          Helen Foster added a comment - MDL-38614 will definitely help the page settings look a lot better! Please note that 'Options' has been renamed 'Display options' in en_fix. I think it's OK having the 2 settings in the section 'Display options'; it keeps General short and consistent with other resources, also the setting 'Display page description' wouldn't look nice next to 'Display description on course page'!
          Hide
          Frédéric Massart added a comment -

          Here is my proposion for Page.

          To keep consistency between modules I thought that we could rename "Options" (or "Display options") to "Appearance".

          Also, now that this section is collapsed by default, I assumed that it was better not to allow the admin to set those settings as advanced, so I've removed this option.

          Show
          Frédéric Massart added a comment - Here is my proposion for Page. To keep consistency between modules I thought that we could rename "Options" (or "Display options") to "Appearance". Also, now that this section is collapsed by default, I assumed that it was better not to allow the admin to set those settings as advanced, so I've removed this option.
          Hide
          Helen Foster added a comment -

          Hi Fred,

          I don't mind if 'Display options' is changed to 'Appearance', though it would need doing for File and URL too (MDL-38004) and also in en_fix. (Let me know if I should do that.)

          Regarding pop-up width and height being made into not-advanced settings, I don't think this is a good idea, as it's removing functionality. I think we have to keep all advanced settings which an admin can set (in IMS content package, URL, File and Page).

          Show
          Helen Foster added a comment - Hi Fred, I don't mind if 'Display options' is changed to 'Appearance', though it would need doing for File and URL too ( MDL-38004 ) and also in en_fix. (Let me know if I should do that.) Regarding pop-up width and height being made into not-advanced settings, I don't think this is a good idea, as it's removing functionality. I think we have to keep all advanced settings which an admin can set (in IMS content package, URL, File and Page).
          Hide
          Frédéric Massart added a comment -

          Hi Helen,

          I didn't think about those URL and File, but I guess that makes sense to make them match the other modules. If we decide to keep "Appearance" then it's probably better if I change it myself to the newly introduced string "appearance" in lang/en/moodle.php, so that we can use one string for every place.

          It's not just about the popup size (which I can keep advance actually), it's about the fact that as the section is rather small, then perhaps it's not worth having some parameters hidden, and if there is a need for it, the admin might not need to be involved in those. Letting the admin controlling the show more/less of a small section could lead to more confusion for the user IMO.

          Though, perhaps we could keep the popup size as advanced, a bit like it's done for File resource (IIRC).

          Show
          Frédéric Massart added a comment - Hi Helen, I didn't think about those URL and File, but I guess that makes sense to make them match the other modules. If we decide to keep "Appearance" then it's probably better if I change it myself to the newly introduced string "appearance" in lang/en/moodle.php, so that we can use one string for every place. It's not just about the popup size (which I can keep advance actually), it's about the fact that as the section is rather small, then perhaps it's not worth having some parameters hidden, and if there is a need for it, the admin might not need to be involved in those. Letting the admin controlling the show more/less of a small section could lead to more confusion for the user IMO. Though, perhaps we could keep the popup size as advanced, a bit like it's done for File resource (IIRC).
          Hide
          Jason Fowler added a comment -

          [Y] Syntax
          [Y] Output
          [Y] Whitespace
          [Y] Language
          [-] Databases
          [Y] Testing
          [-] Security
          [-] Documentation
          [Y] Git
          [Y] Sanity check

          Show
          Jason Fowler added a comment - [Y] Syntax [Y] Output [Y] Whitespace [Y] Language [-] Databases [Y] Testing [-] Security [-] Documentation [Y] Git [Y] Sanity check
          Hide
          Damyon Wiese added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          Thanks!

          Show
          Damyon Wiese added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. Thanks!
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Integrated, thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Integrated, thanks!
          Hide
          Mark Nelson added a comment -

          Works as expected. Passing.

          Show
          Mark Nelson added a comment - Works as expected. Passing.
          Hide
          Dan Poltawski added a comment -

          Did you remember to call thankDevelopers() for 'this_weeks_work'? Defaulting to PARAM_SHODDY thanking.

          line 1289 of \lib\changes.php: call to debugging()
          line 281 of \lib\are.php: call to moodleform->detectMissingThanks()
          line 202 of \lib\now.php: call to moodleform->_is_poor_form()
          line 73 of \course\upstream.php: call to moodleform->forgetingToThank()

          Show
          Dan Poltawski added a comment - Did you remember to call thankDevelopers() for 'this_weeks_work'? Defaulting to PARAM_SHODDY thanking. line 1289 of \lib\changes.php: call to debugging() line 281 of \lib\are.php: call to moodleform->detectMissingThanks() line 202 of \lib\now.php: call to moodleform->_is_poor_form() line 73 of \course\upstream.php: call to moodleform->forgetingToThank()
          Hide
          Mary Cooch added a comment -

          Removing docs_required label as these new display settings are now documented in http://docs.moodle.org/25/en/Page_resource_settings

          Show
          Mary Cooch added a comment - Removing docs_required label as these new display settings are now documented in http://docs.moodle.org/25/en/Page_resource_settings

            People

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

              Dates

              • Created:
                Updated:
                Resolved: