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

      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.

        Gliffy Diagrams

          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: