Moodle
  1. Moodle
  2. MDL-26105

User friendly block settings and help information

    Details

    • Testing Instructions:
      Hide

      How to test:

      Test block settings on different pages (including main course page, front page, module pages etc)
      1. Change 'display page types' and page contexts options to make sure it works as expected
      2. It should work as doc: http://docs.moodle.org/en/Development:Blocks_2.0.3_UI

      Additionally test for peer reviewer, please check if page type pattern displayed if callbacks (such as forum_pagetypelist() in mod/forum/lib.php) not defined. When callbacks not define, generate_page_type_list will try to generate page type list based on current page type, options in blocks settings may display a list like:
      mod-forum-x, mod-forum-view-x...not language strings, but in this case, we cannot find the right language file.

      Show
      How to test: Test block settings on different pages (including main course page, front page, module pages etc) 1. Change 'display page types' and page contexts options to make sure it works as expected 2. It should work as doc: http://docs.moodle.org/en/Development:Blocks_2.0.3_UI Additionally test for peer reviewer, please check if page type pattern displayed if callbacks (such as forum_pagetypelist() in mod/forum/lib.php) not defined. When callbacks not define, generate_page_type_list will try to generate page type list based on current page type, options in blocks settings may display a list like: mod-forum-x, mod-forum-view-x...not language strings, but in this case, we cannot find the right language file.
    • Affected Branches:
      MOODLE_20_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE
    • Pull Master Branch:
      s7_MDL-26105_block_settings_master

      Description

      I'm marking usability aspects now as critical. I think if users don't understand features and can't find any help or explanation the feature won't work for them. That means the feature is not working and this is really a big critical problem.

      The settings for blocks are not user friendly. Terms and functionalities are not explained. Also search at docs don't give any better information:

      Where this block appears: missing help information.
      You can define where the block appears as a default block. Depending from the context where you are adding the block and your own role the options may be different.

      Page context: missing help information
      Display on actual page
      Display on actual page and all subpages
      Display on all page types
      Display on My own MyMoodle page
      Display on all MyMoodle subpages
      Display on ...

      Restrict to these pages types: human unfriendly cryptic description of page types. Really a mess.
      Some of the types have descriptions. Others not. If the cryptic terms are described in the brackets, the cryptic terms are only relevant for developers and can be deleted here.
      Make descriptions understandable.
      delete: course-view-topics write: Any course main page in topics format
      delete: course-view-topics-* write: Any course page in topics course format (Correct?)
      delete: course-view-* write: Any course main page (format independent) (Correct?)
      delete: course-* write: Any course page (format independent) (correct?)
      delete: * write: Any page everywhere
      delete: my-index write: Users 'My' home page
      delete: my-index-* write ? What does it mean?
      delete: my-* write: Any 'My' home pages from any user (Correct?)
      delete: * write: ? What does it mean?

      Specific sub page: missing help information
      Any page matching the above
      This specific page (page 7) ? what menans 'page 7'

      Default region missing help information
      Defines where the block is placed:

      • left left column
      • right right column
      • middle/center area of the page (only if the theme allows this location)

      Default weight Missing help information
      Defines the ordering of the block in the defined region
      -1 on top
      -2 second
      ..
      0 middle
      10 last

      On this page missing help information
      What are these settings for?
      Why can I define visibility on this page if it is predefined that a block is shown at all page types?
      Why is there a different region/weight setting? What happens if the definitions differ?

      Link to documentation page don't contain any of the information for the 2.0 page.

        Gliffy Diagrams

        1. 26105.patch
          7 kB
          Sam Hemelryk
        1. block_my.png
          11 kB
        2. block_my2.png
          6 kB
        3. block_set.png
          16 kB

          Issue Links

            Activity

            Hide
            Helen Foster added a comment -

            Dongsheng, please could you look at the code and explain the meaning of "This specific page (page 7)" when configuring a block on My Moodle.

            Show
            Helen Foster added a comment - Dongsheng, please could you look at the code and explain the meaning of "This specific page (page 7)" when configuring a block on My Moodle.
            Hide
            Dongsheng Cai added a comment -

            Hi, Helen

            page 7 is the subpage, when context id and pagetype ar not enough to uniquely identify a page, we need a subpage id as well, in this specific case, 7 is the primary key of my_pages table.

            Show
            Dongsheng Cai added a comment - Hi, Helen page 7 is the subpage, when context id and pagetype ar not enough to uniquely identify a page, we need a subpage id as well, in this specific case, 7 is the primary key of my_pages table.
            Hide
            Dongsheng Cai added a comment -

            I looked through the code, there are a few places use subpage:
            user/profilesys.php
            user/profile.php
            my/indexsys.php
            my/index.php
            tag/edit.php
            tag/index.php
            mod/lesson/view.php
            mod/lesson/continue.php

            Show
            Dongsheng Cai added a comment - I looked through the code, there are a few places use subpage: user/profilesys.php user/profile.php my/indexsys.php my/index.php tag/edit.php tag/index.php mod/lesson/view.php mod/lesson/continue.php
            Hide
            Helen Foster added a comment -

            Thanks for your comments, though I'm sorry I still don't understand. For My Moodle it sometimes has "This specific page (page 2)" or "This specific page (page 3)". From a user's point of view there is only one My Moodle page. Can't this option be removed?

            Similarly, from a user's point of view they only have one profile page to customize, so can't the option be removed from there too?

            Apart from in the lesson module, I'm really finding it difficult to imagine subpages!

            Show
            Helen Foster added a comment - Thanks for your comments, though I'm sorry I still don't understand. For My Moodle it sometimes has "This specific page (page 2)" or "This specific page (page 3)". From a user's point of view there is only one My Moodle page. Can't this option be removed? Similarly, from a user's point of view they only have one profile page to customize, so can't the option be removed from there too? Apart from in the lesson module, I'm really finding it difficult to imagine subpages!
            Hide
            Ralf Hilgenstock added a comment -

            Hi Dongsheng

            can you explain the concept with a use case a teacher can understand.
            Thanks
            Ralf

            Show
            Ralf Hilgenstock added a comment - Hi Dongsheng can you explain the concept with a use case a teacher can understand. Thanks Ralf
            Hide
            Mary Cooch added a comment -

            I just want to add my +1 to Ralf because I don't understand this either -and even though I wrote about the new block settings in the book I still feel very uneasy explaining them to regular teachers.

            Show
            Mary Cooch added a comment - I just want to add my +1 to Ralf because I don't understand this either -and even though I wrote about the new block settings in the book I still feel very uneasy explaining them to regular teachers.
            Hide
            Dongsheng Cai added a comment -

            Helen

            the 'page 3' stuff can be removed, and use 'This specific page' only.

            Show
            Dongsheng Cai added a comment - Helen the 'page 3' stuff can be removed, and use 'This specific page' only.
            Hide
            Dongsheng Cai added a comment -

            Ralf,

            The subpage pattern is used to identify the specific, when you select that option, the block will appears only on that page, for example, in lesson module, you add a comment block, the select subpage pattern to this specific page, the comment block will appear only on that lesson page, if you jump to another lesson page, you won't see that comment block.

            Regards,
            Dongsheng Cai

            Show
            Dongsheng Cai added a comment - Ralf, The subpage pattern is used to identify the specific, when you select that option, the block will appears only on that page, for example, in lesson module, you add a comment block, the select subpage pattern to this specific page, the comment block will appear only on that lesson page, if you jump to another lesson page, you won't see that comment block. Regards, Dongsheng Cai
            Hide
            Dongsheng Cai added a comment -

            We got some confusing options not only my-* but mod-*

            For example mod-forum-view-* and mod-forum-view has no differences. In module context, mod-* and mod-*-view not working, should be hidden here.

            The reason we have all these confusing options because matching_page_type_patterns generate the page patterns automatically without taking context into account, we probably should add hooks to modules to generate this list.

            For my moodle page, we have my-* my-index-* my-index, there is no difference because we only have my-index, and it is based on user context, we could remove this option, use a default value my-index-*, we will need to fix pagetype pattern in database to make page still being able to find the blocks.

            Show
            Dongsheng Cai added a comment - We got some confusing options not only my-* but mod-* For example mod-forum-view-* and mod-forum-view has no differences. In module context, mod-* and mod-*-view not working, should be hidden here. The reason we have all these confusing options because matching_page_type_patterns generate the page patterns automatically without taking context into account, we probably should add hooks to modules to generate this list. For my moodle page, we have my-* my-index-* my-index, there is no difference because we only have my-index, and it is based on user context, we could remove this option, use a default value my-index-*, we will need to fix pagetype pattern in database to make page still being able to find the blocks.
            Hide
            Dongsheng Cai added a comment -

            I stop the progress because we need a spec to fix this, will have a meeting with Martin when he came back from Japen.

            Show
            Dongsheng Cai added a comment - I stop the progress because we need a spec to fix this, will have a meeting with Martin when he came back from Japen.
            Hide
            Britt added a comment -

            This is causing me trouble. I cannot get the blocks to do what seems logical, so have to just hack at it.

            Show
            Britt added a comment - This is causing me trouble. I cannot get the blocks to do what seems logical, so have to just hack at it.
            Show
            Martin Dougiamas added a comment - Original docs about UI for this (not much): http://docs.moodle.org/en/Development:Very_flexible_block_system_proposal#Blocks_User_interface New spec for this bug: http://docs.moodle.org/en/Development:Blocks_2.0.3_UI
            Hide
            Ray Lawrence added a comment -

            +1 for the comments above. The new flexibility is great but overall rather baffling.

            Show
            Ray Lawrence added a comment - +1 for the comments above. The new flexibility is great but overall rather baffling.
            Hide
            Dongsheng Cai added a comment -

            We managed to make the block page pattern setting minus now, but there are a few places having problems, most of them caused by funny context.

            Blog

            • blog index page using user context, but blog entry page using system context, so if you select "display on all blog pages", it won't work, user context doesn't contain system context, and blog editing page using system context too.
            • Another problem is blog entry page should use other page type pattern like blog-entry not blog-index (fixed in my github branch)

            System and Front page context

            System and Front page context really should have different options, take tag page for example, it using system context, but there are two options about showing it in front page, that doesn't make sense, we cannot simple separate them because inside block lib, 'display on entire moodle site' is changing this option to create sticky blocks. It may causing more confusion, I ended up by adding a hack to hide options for tag.

            Notes

            • Site pages->notes => front page context
            • My profile->notes => user context
            • Course-> Participants => surprisingly, it is system context

            Course report

            The one in Site pages using system context, so page type is course-report
            The reports in course have more types, currently covered by course-report-*

            Local plugin support added, they are using system context

            Question using system and course contexts, it worked fine.

            My code published at https://github.com/dongsheng/moodle/compare/master...s7_MDL-26105_block_settings feel free to test and point out the problems, thanks!

            Show
            Dongsheng Cai added a comment - We managed to make the block page pattern setting minus now, but there are a few places having problems, most of them caused by funny context. Blog blog index page using user context, but blog entry page using system context, so if you select "display on all blog pages", it won't work, user context doesn't contain system context, and blog editing page using system context too. Another problem is blog entry page should use other page type pattern like blog-entry not blog-index (fixed in my github branch) System and Front page context System and Front page context really should have different options, take tag page for example, it using system context, but there are two options about showing it in front page, that doesn't make sense, we cannot simple separate them because inside block lib, 'display on entire moodle site' is changing this option to create sticky blocks. It may causing more confusion, I ended up by adding a hack to hide options for tag. Notes Site pages->notes => front page context My profile->notes => user context Course-> Participants => surprisingly, it is system context Course report The one in Site pages using system context, so page type is course-report The reports in course have more types, currently covered by course-report-* Local plugin support added, they are using system context Question using system and course contexts, it worked fine. My code published at https://github.com/dongsheng/moodle/compare/master...s7_MDL-26105_block_settings feel free to test and point out the problems, thanks!
            Hide
            Dongsheng Cai added a comment -

            repo: git@github.com:dongsheng/moodle.git
            branch: s7_MDL-26105_block_settings
            diff: https://github.com/dongsheng/moodle/compare/master...s7_MDL-26105_block_settings

            Show
            Dongsheng Cai added a comment - repo: git@github.com:dongsheng/moodle.git branch: s7_ MDL-26105 _block_settings diff: https://github.com/dongsheng/moodle/compare/master...s7_MDL-26105_block_settings
            Hide
            Martin Dougiamas added a comment -

            Help files for:

            Original block location: This is the original location where the block was created, even though other block settings may cause it to appear in other locations (contexts) within this original location. For example, a block created in a course could be displayed on one or more pages within that course only. A block created on the front page, however, could be displayed throughout the site.

            Page contexts: Contexts are more specific types of pages where this block can be displayed within the original block location. You will have different options here depending on the original block location and your current location. For example, you can restrict a block to only appearing on forum pages in a course by adding the block to the course (making it appear on all sub-pages), then going into a forum and editing the block settings again to restrict display to just forum pages.

            Default region: Themes may define one or more named block regions where blocks are displayed. This setting defines which of these you want this block to appear in by default. The region may be overridden on specific pages if required.

            Default weight: The default weight allows you to choose roughly where you want the block to appear in the chosen region, either at the top or the bottom. The final location is calculated from all the blocks in that region (for example, only one block can actually be at the top). This value can be overridden on specific pages if required.

            Show
            Martin Dougiamas added a comment - Help files for: Original block location: This is the original location where the block was created, even though other block settings may cause it to appear in other locations (contexts) within this original location. For example, a block created in a course could be displayed on one or more pages within that course only. A block created on the front page, however, could be displayed throughout the site. Page contexts: Contexts are more specific types of pages where this block can be displayed within the original block location. You will have different options here depending on the original block location and your current location. For example, you can restrict a block to only appearing on forum pages in a course by adding the block to the course (making it appear on all sub-pages), then going into a forum and editing the block settings again to restrict display to just forum pages. Default region: Themes may define one or more named block regions where blocks are displayed. This setting defines which of these you want this block to appear in by default. The region may be overridden on specific pages if required. Default weight: The default weight allows you to choose roughly where you want the block to appear in the chosen region, either at the top or the bottom. The final location is calculated from all the blocks in that region (for example, only one block can actually be at the top). This value can be overridden on specific pages if required.
            Hide
            Aparup Banerjee added a comment -

            just noting that there seems to be something funny going on with the help popups under block settings page. DS is looking into it.

            Show
            Aparup Banerjee added a comment - just noting that there seems to be something funny going on with the help popups under block settings page. DS is looking into it.
            Hide
            Dongsheng Cai added a comment -

            Filed a bug about the help button: MDL-27111, it seems not related to block system.

            Show
            Dongsheng Cai added a comment - Filed a bug about the help button: MDL-27111 , it seems not related to block system.
            Hide
            Aparup Banerjee added a comment - - edited

            Hi DS,

            i've come across a slight confusion:
            in a single discussion type forum, the block must be specified as being on the main page to be shown in the discussion (which is the main page in this case), but if i select 'Forum module discuss page' it doesn't show up in the discussion. Maybe we should reduce the choices in this case?

            maybe assignment could have a few more block page options (submissions, grading etc)?

            otherwise all seems fine to me

            Show
            Aparup Banerjee added a comment - - edited Hi DS, i've come across a slight confusion: in a single discussion type forum, the block must be specified as being on the main page to be shown in the discussion (which is the main page in this case), but if i select 'Forum module discuss page' it doesn't show up in the discussion. Maybe we should reduce the choices in this case? maybe assignment could have a few more block page options (submissions, grading etc)? otherwise all seems fine to me
            Hide
            Dongsheng Cai added a comment -

            pull request submitted.

            Show
            Dongsheng Cai added a comment - pull request submitted.
            Hide
            Dongsheng Cai added a comment -

            Submitted two pull requests PULL-678 PULL-679, moved languages from pagetype.php to module language files.

            I am not sure of using frankenstyle in generate_page_type_patterns() because pagetype name is reliable.

            Show
            Dongsheng Cai added a comment - Submitted two pull requests PULL-678 PULL-679, moved languages from pagetype.php to module language files. I am not sure of using frankenstyle in generate_page_type_patterns() because pagetype name is reliable.
            Hide
            Dongsheng Cai added a comment -

            Mark as resolved for further review.

            Show
            Dongsheng Cai added a comment - Mark as resolved for further review.
            Hide
            Dongsheng Cai added a comment -

            Comment from Sam:

            Hi Dongsheng,

            I'm rejecting this because it is presently breaking backwards compatibility.
            Try adding blocks to a module that hasn't defined the modname_pagetypelist function. The block disappears.
            The problem appears to be that an array of int => pattern is returned from the generate function rather than pattern => description.
            Looking at the code this would also affect all areas that don't have a specific if~branch to handle them.

            The following are minor issues I also noticed:

            There are two functions missing phpdocs
            Several phpdoc blocks use object, please change that to stdClass (@param object $blah == @param stdClass $blah)
            Is the note if a typo? Should that be notes

            Seeing as this has to be revisited again anyway perhaps you could fix up the string Petr noted as well, looks like it would just be one more branch in your if's tree.

            Actually I still don't like the ifs to deal with specific components within generate_page_type_patterns. I've been having a think about this and I can't see a good way around it.
            If you change to frankenstyle it'd be easy but you'd break all theme's in the process.
            If you don't change that if..else..if.else code is just going to grow and grow.
            Perhaps for this fix you should change that to a switch statement so it grows in a cleaner way and then create a new MDL issue to see a solution developed to use frankenstyle that also tries to keen things backwards compatible.
            Might be best to talk to Petr about that given he wrote a lot of the theme/page handling code.

            Cheers
            Sam

            Show
            Dongsheng Cai added a comment - Comment from Sam: Hi Dongsheng, I'm rejecting this because it is presently breaking backwards compatibility. Try adding blocks to a module that hasn't defined the modname_pagetypelist function. The block disappears. The problem appears to be that an array of int => pattern is returned from the generate function rather than pattern => description. Looking at the code this would also affect all areas that don't have a specific if~branch to handle them. The following are minor issues I also noticed: There are two functions missing phpdocs Several phpdoc blocks use object, please change that to stdClass (@param object $blah == @param stdClass $blah) Is the note if a typo? Should that be notes Seeing as this has to be revisited again anyway perhaps you could fix up the string Petr noted as well, looks like it would just be one more branch in your if's tree. Actually I still don't like the ifs to deal with specific components within generate_page_type_patterns. I've been having a think about this and I can't see a good way around it. If you change to frankenstyle it'd be easy but you'd break all theme's in the process. If you don't change that if..else..if.else code is just going to grow and grow. Perhaps for this fix you should change that to a switch statement so it grows in a cleaner way and then create a new MDL issue to see a solution developed to use frankenstyle that also tries to keen things backwards compatible. Might be best to talk to Petr about that given he wrote a lot of the theme/page handling code. Cheers Sam
            Hide
            Dongsheng Cai added a comment -

            I fixed generate_page_type_patterns for following problem:
            1. Generate pattern => description array if callbacks not defined or not covered by branches
            2. removed if-else code, replaced by switch-case

            Added phpdocs to generate_page_type_patterns and user_pagetypelist, replaced object type to stdClass type in phpdocs

            note_pagetypelsit is not a typo, I noticed other notes functions use the "note_" prefix, better to keep consistence, right?

            Show
            Dongsheng Cai added a comment - I fixed generate_page_type_patterns for following problem: 1. Generate pattern => description array if callbacks not defined or not covered by branches 2. removed if-else code, replaced by switch-case Added phpdocs to generate_page_type_patterns and user_pagetypelist, replaced object type to stdClass type in phpdocs note_pagetypelsit is not a typo, I noticed other notes functions use the "note_" prefix, better to keep consistence, right?
            Hide
            Dongsheng Cai added a comment -

            Added Aparup for peer review. There is no major changes since last peer review, just improvement on generate_page_type_list, fixed a bug of gerneate_page_type_list if plugin callback not defined, and other minor fixes on phpdocs.

            Show
            Dongsheng Cai added a comment - Added Aparup for peer review. There is no major changes since last peer review, just improvement on generate_page_type_list, fixed a bug of gerneate_page_type_list if plugin callback not defined, and other minor fixes on phpdocs.
            Hide
            Aparup Banerjee added a comment -

            Hi DS,
            1) suggest to have bui_contexts values as constants for easier read
            2) around '// Generate page type patterns based on current page type if' theres a trailing whitespace
            3) i wondered near the local plugin callback .. return $functionname($pagetype, ...., do we want to handle if the function doesn't exist or risk fatal err?

            all else seems fine to me.

            Show
            Aparup Banerjee added a comment - Hi DS, 1) suggest to have bui_contexts values as constants for easier read 2) around '// Generate page type patterns based on current page type if' theres a trailing whitespace 3) i wondered near the local plugin callback .. return $functionname($pagetype, ...., do we want to handle if the function doesn't exist or risk fatal err? all else seems fine to me.
            Hide
            Dongsheng Cai added a comment -

            Thanks Aparup

            1. I added constants for each options
            2. whitespace removed
            3. Good point, added function_exists check, if not, the following code will generate page list based on current page type.
            Show
            Dongsheng Cai added a comment - Thanks Aparup I added constants for each options whitespace removed Good point, added function_exists check, if not, the following code will generate page list based on current page type.
            Hide
            Aparup Banerjee added a comment -

            Code looks fine to me. it works for me!

            Show
            Aparup Banerjee added a comment - Code looks fine to me. it works for me!
            Hide
            Sam Hemelryk added a comment -

            Hi Dongsheng,

            I've been looking at this for the past couple of hours now.
            I am going to wait till Eloy comes online today and talk to him about this, there are a couple of things about this patch that I am conflicted about all of which fall back to that switch statement:
            While playing around with it became very apparent that any areas not dealt with explicitly often have problems, e.g. with the patch applied you can no longer add blocks to course report pages.
            I started having a play with things and I've attached a patch that you can apply ontop of your current branch that replaces the switch statement with code that looks up the location using the get_core and get plugin methods. However that is still not a completely working solution either as components where the component name differs from the path are not resolvable in this fashion.
            The more I look at this the more it becomes apparent that we either break functionality and implement this or break themes and fix once and for all by converting pagetypes to use frankenstyle like everything else.
            Either way we should discuss this with Eloy and see what he thinks.

            Cheers
            Sam

            Show
            Sam Hemelryk added a comment - Hi Dongsheng, I've been looking at this for the past couple of hours now. I am going to wait till Eloy comes online today and talk to him about this, there are a couple of things about this patch that I am conflicted about all of which fall back to that switch statement: While playing around with it became very apparent that any areas not dealt with explicitly often have problems, e.g. with the patch applied you can no longer add blocks to course report pages. I started having a play with things and I've attached a patch that you can apply ontop of your current branch that replaces the switch statement with code that looks up the location using the get_core and get plugin methods. However that is still not a completely working solution either as components where the component name differs from the path are not resolvable in this fashion. The more I look at this the more it becomes apparent that we either break functionality and implement this or break themes and fix once and for all by converting pagetypes to use frankenstyle like everything else. Either way we should discuss this with Eloy and see what he thinks. Cheers Sam
            Hide
            Dongsheng Cai added a comment -

            Thanks for the patch, Sam.

            I applied your patch to my banches. Please update.

            Regards,
            Dongsheng

            Show
            Dongsheng Cai added a comment - Thanks for the patch, Sam. I applied your patch to my banches. Please update. Regards, Dongsheng
            Hide
            Sam Hemelryk added a comment -

            Hi Dongsheng,

            I'm reopening this so that more work can be done to fix up the regressions for outside cases such as course reports and so that more investigation can be done into other fringe areas.
            I have also asked Eloy to have a look at the patch and see if he has any good ideas about how to locate callback methods.

            Cheers
            Sam

            Show
            Sam Hemelryk added a comment - Hi Dongsheng, I'm reopening this so that more work can be done to fix up the regressions for outside cases such as course reports and so that more investigation can be done into other fringe areas. I have also asked Eloy to have a look at the patch and see if he has any good ideas about how to locate callback methods. Cheers Sam
            Hide
            Ray Lawrence added a comment -

            An appeal for help when this issue is resolved.

            There appears to be a number of subtle changes to the behaviour of these settings. As control of blocks is fundamental in course building, please can you document the actual behaviour (taking account of all relevant contexts) in MoodleDocs?

            A quick question too, are these aborted changes present in the latest weekly release?

            Show
            Ray Lawrence added a comment - An appeal for help when this issue is resolved. There appears to be a number of subtle changes to the behaviour of these settings. As control of blocks is fundamental in course building, please can you document the actual behaviour (taking account of all relevant contexts) in MoodleDocs? A quick question too, are these aborted changes present in the latest weekly release?
            Hide
            Helen Foster added a comment -

            Ray, as this issue has been reopened, the changes will NOT be included in this week's release.

            Show
            Helen Foster added a comment - Ray, as this issue has been reopened, the changes will NOT be included in this week's release.
            Hide
            Ray Lawrence added a comment -

            Thanks. But they are in the current one i.e. last week? I'm just trying to work out what it is I'm seeing - original, reworked, original re-instated, patched. Help!

            Show
            Ray Lawrence added a comment - Thanks. But they are in the current one i.e. last week? I'm just trying to work out what it is I'm seeing - original, reworked, original re-instated, patched. Help!
            Hide
            Helen Foster added a comment -

            No, they're not in the current one either. Changes are only added if the code passes an integration review and testing.

            Show
            Helen Foster added a comment - No, they're not in the current one either. Changes are only added if the code passes an integration review and testing.
            Hide
            Ray Lawrence added a comment -

            Thanks

            Show
            Ray Lawrence added a comment - Thanks
            Hide
            Martin Dougiamas added a comment -

            Yes, we definitely need to highlight this issue as an interface change in the release notes and in docs

            Show
            Martin Dougiamas added a comment - Yes, we definitely need to highlight this issue as an interface change in the release notes and in docs
            Hide
            Martin Dougiamas added a comment -

            Please let's not extend the issue unecessarily ... it really has to be fixed from the GUI pov ASAP. If backward compatibility of themes is not affected at the moment and the key areas areas are all fixed then it's better than it was and should get in.

            Show
            Martin Dougiamas added a comment - Please let's not extend the issue unecessarily ... it really has to be fixed from the GUI pov ASAP. If backward compatibility of themes is not affected at the moment and the key areas areas are all fixed then it's better than it was and should get in.
            Hide
            Andrew Davis added a comment - - edited

            Ive started this and have found some problems.

            Blocks in blogs seem to be pretty broken. Selecting "any page" or "all blog pages" seems to have the same effect as selecting "blog listing pages". The block never appears anywhere but the blog listing page. Selecting "blog editing pages" removes the block from the listing page but it doesnt appear on the blog entry editing page so it effectively removes the block.

            If you add a block to a course then set "display on page types" to "any activity module page" the block disappears from the course which is expected (I believe). However when you go into an activity you get php errors. I'm trying to fix these now.

            Show
            Andrew Davis added a comment - - edited Ive started this and have found some problems. Blocks in blogs seem to be pretty broken. Selecting "any page" or "all blog pages" seems to have the same effect as selecting "blog listing pages". The block never appears anywhere but the blog listing page. Selecting "blog editing pages" removes the block from the listing page but it doesnt appear on the blog entry editing page so it effectively removes the block. If you add a block to a course then set "display on page types" to "any activity module page" the block disappears from the course which is expected (I believe). However when you go into an activity you get php errors. I'm trying to fix these now.
            Hide
            Andrew Davis added a comment -

            Ive added another commit to the version in my repository. https://github.com/andyjdavis/moodle/compare/master...s7_MDL-26105_block_settings_master

            That fixes the php errors plus I made one of the help strings, hopefully, a little clearer.

            I'll finish testing tomorrow.

            Show
            Andrew Davis added a comment - Ive added another commit to the version in my repository. https://github.com/andyjdavis/moodle/compare/master...s7_MDL-26105_block_settings_master That fixes the php errors plus I made one of the help strings, hopefully, a little clearer. I'll finish testing tomorrow.
            Hide
            Andrew Davis added a comment -

            Updating the repo, branch and diff details. I've fixed a few issues. This really needs to get integrated so I'm going to document the problems I've identified during my testing in other MDLs which I will link to this one.

            Show
            Andrew Davis added a comment - Updating the repo, branch and diff details. I've fixed a few issues. This really needs to get integrated so I'm going to document the problems I've identified during my testing in other MDLs which I will link to this one.
            Hide
            Andrew Davis added a comment -

            I have created 5 linked issues.

            Show
            Andrew Davis added a comment - I have created 5 linked issues.
            Hide
            Sam Hemelryk added a comment -

            Need to talk to Martin about the current behaviour of this patch on areas that don't have explicit callbacks for definitions.

            Show
            Sam Hemelryk added a comment - Need to talk to Martin about the current behaviour of this patch on areas that don't have explicit callbacks for definitions.
            Hide
            Sam Hemelryk added a comment -

            Hi guys,

            As discussed in the office this issue has now been integrated to master.
            I will be opening two blockers immediately after this as follows:

            1. To fix case of overlay between core components and plugins (for Andrew)
            2. To see this backported to MOODLE_20_STABLE is desired after the above issue has been resolved and things have been properly tested.

            Cheers
            Sam

            Show
            Sam Hemelryk added a comment - Hi guys, As discussed in the office this issue has now been integrated to master. I will be opening two blockers immediately after this as follows: To fix case of overlay between core components and plugins (for Andrew) To see this backported to MOODLE_20_STABLE is desired after the above issue has been resolved and things have been properly tested. Cheers Sam
            Hide
            Sam Hemelryk added a comment -

            Have created MDL-27829 to see the overlap bug fixed and MDL-27830 to see it considered for backport to 2.0

            Show
            Sam Hemelryk added a comment - Have created MDL-27829 to see the overlap bug fixed and MDL-27830 to see it considered for backport to 2.0
            Hide
            Helen Foster added a comment -

            There seems to be a problem with how the Block settings help popups open. Elsewhere in Moodle help popups open in a small window, keeping the user on the same page. Block settings help popups take the user to a new page containing the help string.

            Hope this problem can be quickly fixed.

            Show
            Helen Foster added a comment - There seems to be a problem with how the Block settings help popups open. Elsewhere in Moodle help popups open in a small window, keeping the user on the same page. Block settings help popups take the user to a new page containing the help string. Hope this problem can be quickly fixed.
            Hide
            Helen Foster added a comment -

            When attempting to edit a new HTML block on the My home page, I obtain a couple of notices

            Notice: Undefined property: stdClass::$text in /html/blocks/html/edit_form.php on line 47
            Notice: Undefined property: stdClass::$format in /html/blocks/html/edit_form.php on line 56

            (Sorry I don't know whether they're relevant or not!)

            Also, the 'Display on page types' setting includes the options my-index, my-index-* and my-*. I can't quite figure out from http://docs.moodle.org/dev/Blocks_2.0.3_UI how it should be, but I thought this issue was about providing more user-friendly settings as Ralf suggested?

            Show
            Helen Foster added a comment - When attempting to edit a new HTML block on the My home page, I obtain a couple of notices Notice: Undefined property: stdClass::$text in /html/blocks/html/edit_form.php on line 47 Notice: Undefined property: stdClass::$format in /html/blocks/html/edit_form.php on line 56 (Sorry I don't know whether they're relevant or not!) Also, the 'Display on page types' setting includes the options my-index, my-index-* and my-*. I can't quite figure out from http://docs.moodle.org/dev/Blocks_2.0.3_UI how it should be, but I thought this issue was about providing more user-friendly settings as Ralf suggested?
            Hide
            Andrew Davis added a comment -

            I dont think the errors with html block are related although they are odd. Im not able to get it to happen.

            I can reproduce the help popup issue although Im currently at a loss as to why/how the popups are not working on that specific page.

            re this
            "Also, the 'Display on page types' setting includes the options my-index, my-index-* and my-*"
            where are you seeing that? Can you send me the URL of the page that youve added the block to. You shouldnt be seeing those kinds of options anywhere.

            Show
            Andrew Davis added a comment - I dont think the errors with html block are related although they are odd. Im not able to get it to happen. I can reproduce the help popup issue although Im currently at a loss as to why/how the popups are not working on that specific page. re this "Also, the 'Display on page types' setting includes the options my-index, my-index-* and my-*" where are you seeing that? Can you send me the URL of the page that youve added the block to. You shouldnt be seeing those kinds of options anywhere.
            Hide
            Helen Foster added a comment -

            Andrew, I'm using http://qa.moodle.net for testing this issue. I obtained the notices when logged in as an admin. I just tried logging in as a manager and found no notices. (Please PM me if you need the admin password to test.)

            I obtained the options my-index, my-index-* and my-* when configuring a block on http://qa.moodle.net/my/index.php (logged in as admin or manager).

            I also obtained similar options notes-index, notes-index-* and notes-* when configuring a block on http://qa.moodle.net/notes/index.php.

            Show
            Helen Foster added a comment - Andrew, I'm using http://qa.moodle.net for testing this issue. I obtained the notices when logged in as an admin. I just tried logging in as a manager and found no notices. (Please PM me if you need the admin password to test.) I obtained the options my-index, my-index-* and my-* when configuring a block on http://qa.moodle.net/my/index.php (logged in as admin or manager). I also obtained similar options notes-index, notes-index-* and notes-* when configuring a block on http://qa.moodle.net/notes/index.php .
            Hide
            Andrew Davis added a comment -

            Ive been able to fix the problem with the help popups with help from Sam.

            Show
            Andrew Davis added a comment - Ive been able to fix the problem with the help popups with help from Sam.
            Hide
            Andrew Davis added a comment -

            I have fixed the issue with blocks on the my moodle page. Im going to open a new issue for the notes page as this is becoming complicated. The issue with the my moodle page was introduced when an empty function was stripped out during integration. The function shouldnt have been empty however the error handling was making it all work. Removing that function made it not work.

            So while the original branch is based on the main repository I've had to do this fix in a branch based on integration. Eloy, the branch based on integration is called MDL-26105_block_settings_2. The two commits we need are:
            https://github.com/andyjdavis/moodle/commit/9672689e3bc4de6b5b6be1e09e84a538ab3f75d6
            https://github.com/andyjdavis/moodle/commit/f00912b9d806e404de46cc207105274cdaf8d8fa

            Is it possible for you to also cherry pick these commits into 2.0 stable?

            once this is all bedded down Ill get back to MDL-27829. That depends on this :|

            Show
            Andrew Davis added a comment - I have fixed the issue with blocks on the my moodle page. Im going to open a new issue for the notes page as this is becoming complicated. The issue with the my moodle page was introduced when an empty function was stripped out during integration. The function shouldnt have been empty however the error handling was making it all work. Removing that function made it not work. So while the original branch is based on the main repository I've had to do this fix in a branch based on integration. Eloy, the branch based on integration is called MDL-26105 _block_settings_2. The two commits we need are: https://github.com/andyjdavis/moodle/commit/9672689e3bc4de6b5b6be1e09e84a538ab3f75d6 https://github.com/andyjdavis/moodle/commit/f00912b9d806e404de46cc207105274cdaf8d8fa Is it possible for you to also cherry pick these commits into 2.0 stable? once this is all bedded down Ill get back to MDL-27829 . That depends on this :|
            Hide
            Andrew Davis added a comment -

            Sorry about that. Some code for an unrelated item somehow wound up in one of those commits.

            Branch: MDL-26105_block_settings_once_more_unto_the_breach

            Commits:
            https://github.com/andyjdavis/moodle/commit/65707c3a16d9da0b8cd544e88612a2721c7fd246
            https://github.com/andyjdavis/moodle/commit/50565f564dcc3289078910602817879d37514ed1

            Show
            Andrew Davis added a comment - Sorry about that. Some code for an unrelated item somehow wound up in one of those commits. Branch: MDL-26105 _block_settings_once_more_unto_the_breach Commits: https://github.com/andyjdavis/moodle/commit/65707c3a16d9da0b8cd544e88612a2721c7fd246 https://github.com/andyjdavis/moodle/commit/50565f564dcc3289078910602817879d37514ed1
            Hide
            Tim Hunt added a comment -

            The need for https://github.com/andyjdavis/moodle/commit/50565f564dcc3289078910602817879d37514ed1 is really complicated, but it is correct:

            (10:47:05 AM) david: Eloy: why is $output = $editpage->get_renderer('core'); needed?
            (10:47:40 AM) timhunt: Renderers are specific to the page they are rendering onto (becuase theme depends on page, and renderer depends on theme)
            (10:47:59 AM) timhunt: So, if you change $PAGE, after $OUPTUT has been set up, you also need ot re-set-up $OUTPUT>
            (10:48:25 AM) Eloy: that's so a dark-zone in my knowledge... grrr
            (10:48:40 AM) timhunt: Except that, If you have already used $OUTPUT (from one theme, for example) then it is really bad to change to a different $OUTPUT half way through the output.
            (10:49:37 AM) david: that worries me
            (10:49:54 AM) timhunt: The call to $this->blocks->process_url_actions() - which is what triggers this - is in $PAGE->starting_output(); which is what sets this up.
            (10:51:13 AM) timhunt: But, that is not where $OUTPUT is created.
            (10:52:40 AM) timhunt: So, the normal flow is this:
            (10:53:07 AM) aparup left the room.
            (10:53:55 AM) timhunt: 1. First call to $OUTPUT is $OUTPUT->header, which is actally a call to bootstrap_renderer::header() which initialises $OUTPUT, calls core_rendere->header(), which does $this->page->set_state(moodle_page::STATE_PRINTING_HEADER); which gets to the code we are reviewing.
            (10:55:09 AM) timhunt: So, in process_url_edit, where we are throwing away the current page, and starting again, it is correct to re-create $OUTPUT.
            (10:55:40 AM) timhunt: However, all this code is too bloody complicated, and the person who wrote it should be sent back to the OU where he can only bugger up the question bank
            (10:56:35 AM) Eloy: lol

            So, +1 from me.

            Show
            Tim Hunt added a comment - The need for https://github.com/andyjdavis/moodle/commit/50565f564dcc3289078910602817879d37514ed1 is really complicated, but it is correct: (10:47:05 AM) david: Eloy: why is $output = $editpage->get_renderer('core'); needed? (10:47:40 AM) timhunt: Renderers are specific to the page they are rendering onto (becuase theme depends on page, and renderer depends on theme) (10:47:59 AM) timhunt: So, if you change $PAGE, after $OUPTUT has been set up, you also need ot re-set-up $OUTPUT> (10:48:25 AM) Eloy: that's so a dark-zone in my knowledge... grrr (10:48:40 AM) timhunt: Except that, If you have already used $OUTPUT (from one theme, for example) then it is really bad to change to a different $OUTPUT half way through the output. (10:49:37 AM) david: that worries me (10:49:54 AM) timhunt: The call to $this->blocks->process_url_actions() - which is what triggers this - is in $PAGE->starting_output(); which is what sets this up. (10:51:13 AM) timhunt: But, that is not where $OUTPUT is created. (10:52:40 AM) timhunt: So, the normal flow is this: (10:53:07 AM) aparup left the room. (10:53:55 AM) timhunt: 1. First call to $OUTPUT is $OUTPUT->header, which is actally a call to bootstrap_renderer::header() which initialises $OUTPUT, calls core_rendere->header(), which does $this->page->set_state(moodle_page::STATE_PRINTING_HEADER); which gets to the code we are reviewing. (10:55:09 AM) timhunt: So, in process_url_edit, where we are throwing away the current page, and starting again, it is correct to re-create $OUTPUT. (10:55:40 AM) timhunt: However, all this code is too bloody complicated, and the person who wrote it should be sent back to the OU where he can only bugger up the question bank (10:56:35 AM) Eloy: lol So, +1 from me.
            Hide
            Eloy Lafuente (stronk7) added a comment -

            Many thanks Tim, integrating now.

            Show
            Eloy Lafuente (stronk7) added a comment - Many thanks Tim, integrating now.
            Hide
            Eloy Lafuente (stronk7) added a comment -

            This has been integrated now, thanks!

            NOTE: This is master only. As commented by SamH above.. backport to 20_STABLE should / will happen in different MDL issue: MDL-27830

            Show
            Eloy Lafuente (stronk7) added a comment - This has been integrated now, thanks! NOTE: This is master only. As commented by SamH above.. backport to 20_STABLE should / will happen in different MDL issue: MDL-27830
            Hide
            Helen Foster added a comment -

            Not sure whether this is relevant (or perhaps I'm doing something wrong!), but I just tried configuring the comments block on http://qa.moodle.net to display throughout the site, however it wouldn't display on a course page.

            Show
            Helen Foster added a comment - Not sure whether this is relevant (or perhaps I'm doing something wrong!), but I just tried configuring the comments block on http://qa.moodle.net to display throughout the site, however it wouldn't display on a course page.
            Hide
            Eloy Lafuente (stronk7) added a comment - - edited

            Passing, although really I don't know if it's working as expected, for example in the /my page. See HQ chat (conversationid=7657) some minutes ago.

            Ciao

            Edited: Added ref to HQ chat.

            Show
            Eloy Lafuente (stronk7) added a comment - - edited Passing, although really I don't know if it's working as expected, for example in the /my page. See HQ chat (conversationid=7657) some minutes ago. Ciao Edited: Added ref to HQ chat.
            Hide
            Eloy Lafuente (stronk7) added a comment -

            And now this is part of the best Moodle weeklies ever, thanks!

            Closing. Plz, review /my and other possible multi-pages places, I really didn't know what/how to test there.

            Show
            Eloy Lafuente (stronk7) added a comment - And now this is part of the best Moodle weeklies ever, thanks! Closing. Plz, review /my and other possible multi-pages places, I really didn't know what/how to test there.
            Hide
            Gareth J Barnard added a comment - - edited

            Hi,

            I'm trying to find in Moodle 2.1.3 where the image block_set.png is within Moodle so that I can see if I need to add the following to my Course format's language files:

            $string['page-course-view-topcoll'] = 'Any course main page in collapsed topics format';
            $string['page-course-view-topcoll-x'] = 'Any course page in collapsed topics format';

            Please could you let me know if they are required and if so how to test.

            Thanks,

            Gareth

            Show
            Gareth J Barnard added a comment - - edited Hi, I'm trying to find in Moodle 2.1.3 where the image block_set.png is within Moodle so that I can see if I need to add the following to my Course format's language files: $string ['page-course-view-topcoll'] = 'Any course main page in collapsed topics format'; $string ['page-course-view-topcoll-x'] = 'Any course page in collapsed topics format'; Please could you let me know if they are required and if so how to test. Thanks, Gareth
            Hide
            Helen Foster added a comment -

            Gareth, sorry I can't help you, but how about posting your question in one of the forums in Using Moodle, as I don't think devs spend much time looking at closed bugs!

            Show
            Helen Foster added a comment - Gareth, sorry I can't help you, but how about posting your question in one of the forums in Using Moodle, as I don't think devs spend much time looking at closed bugs!
            Hide
            Gareth J Barnard added a comment -

            Dear Helen,

            Thank you, I did not consider that . I have eMailed Andrew and see if I get a response. I tried to figure out the code in both Moodle 2.0 and 2.1 but could not determine exactly how it works. I tend to keep my formats up to date with developments by monitoring changes through Git and spotted this the other day.

            Cheers,

            Gareth

            Show
            Gareth J Barnard added a comment - Dear Helen, Thank you, I did not consider that . I have eMailed Andrew and see if I get a response. I tried to figure out the code in both Moodle 2.0 and 2.1 but could not determine exactly how it works. I tend to keep my formats up to date with developments by monitoring changes through Git and spotted this the other day. Cheers, Gareth

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved: