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
    • Rank:
      15690

      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.

      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: