Moodle
  1. Moodle
  2. MDL-33890

Drag n Drop does not work when moving blocks into custom regions

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.3
    • Fix Version/s: 2.3.2
    • Component/s: Administration
    • Labels:
    • Testing Instructions:
      Hide
      1. Install Aardvark Post-IT theme with the patch
      2. Move block to new block region at the top
      3. Move block from the new region (it becomes empty) - make sure that side block regions are not getting hidden at this point.

      Also, testing existing functionality is required:

      1. Move all blocks from left or right region that does not contain block adding block.
      2. On the last block move, the region should be hidden.
      3. Move any block towards the hidden region, the region should be displayed and accept new block.
      Show
      Install Aardvark Post-IT theme with the patch Move block to new block region at the top Move block from the new region (it becomes empty) - make sure that side block regions are not getting hidden at this point. Also, testing existing functionality is required: Move all blocks from left or right region that does not contain block adding block. On the last block move, the region should be hidden. Move any block towards the hidden region, the region should be displayed and accept new block.
    • Workaround:
      Hide

      Switch Javascript off in the browser and move them the old way.

      Show
      Switch Javascript off in the browser and move them the old way.
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-33890-master-2
    • Rank:
      41992

      Description

      In contributed themes like Aardvark Post-IT and Aerie which have custom block-regions set in the layout, the new AJAX Drag n Drop feature does not allow you to move blocks into these designated areas.

      On further inspection of this problem, it has come to light that the main 'block-regions' like 'region-pre' snd 'region-post' have been hard coded in lib/yui/blocks/blocks.js

       var regionid = this.get_region_id(blockregionlist.item(0));
                      if (regionid === 'post') {
                          // pre block is missing, instert it before post
                          blockregion.setAttrs({id : 'region-pre'});
                          blockregionlist.item(0).insert(blockregion, 'before');
                          blockregionlist.unshift(blockregion);
                      } else {
                          // post block is missing, instert it after pre
                          blockregion.setAttrs({id : 'region-post'});
                          blockregionlist.item(0).insert(blockregion, 'after');
                          blockregionlist.push(blockregion);
                      }
      

        Issue Links

          Activity

          Hide
          Mary Evans added a comment -

          @Rajesh

          Sorry if I've got this in the wrong component area, please feel free to assign it to someone else.

          However, I'm just wondering if there is a quick way to fix this by adding Javascript which extends ../lib/yui/blocks/blocks.js, in the theme itself?

          Would that be possible? If so HOW?

          Thanks Mary

          Show
          Mary Evans added a comment - @Rajesh Sorry if I've got this in the wrong component area, please feel free to assign it to someone else. However, I'm just wondering if there is a quick way to fix this by adding Javascript which extends ../lib/yui/blocks/blocks.js, in the theme itself? Would that be possible? If so HOW? Thanks Mary
          Hide
          Dan Poltawski added a comment -

          Assigning this to Ruslan who is the expert here.

          Show
          Dan Poltawski added a comment - Assigning this to Ruslan who is the expert here.
          Hide
          Dan Poltawski added a comment -

          Actually I think this is a duplicate of MDL-32608?

          Show
          Dan Poltawski added a comment - Actually I think this is a duplicate of MDL-32608 ?
          Hide
          Mary Evans added a comment -

          I don't think this is the same thing, but it is related I grant you that.
          There is nothing wrong with moving block regions side to side, so I am not sure what that discussion in MDL-32608 was all about, however moving blocks to new block-regions is a whole new ball game.

          Show
          Mary Evans added a comment - I don't think this is the same thing, but it is related I grant you that. There is nothing wrong with moving block regions side to side, so I am not sure what that discussion in MDL-32608 was all about, however moving blocks to new block-regions is a whole new ball game.
          Hide
          Mary Evans added a comment -

          @Dan

          This is more related to MDL-32652

          Show
          Mary Evans added a comment - @Dan This is more related to MDL-32652
          Hide
          Ruslan Kabalin added a comment -

          The code in description is not related to the issue, as this simply creates new placeholders for the missing default block areas.

          But indeed there might be a problem with custom themes that introduce new block regions. On the other hand, I do not think I should test it with every single custom theme and propose fixes. In this particular example, if new block region of custom theme can be captured with '#page-content div.block-region', it should work, but it looks like it uses different CSS selectors. Theme author should try to adjust the theme first, if this will not work for some reason, then I will have a look and we decide how to make the feature more flexible.

          Show
          Ruslan Kabalin added a comment - The code in description is not related to the issue, as this simply creates new placeholders for the missing default block areas. But indeed there might be a problem with custom themes that introduce new block regions. On the other hand, I do not think I should test it with every single custom theme and propose fixes. In this particular example, if new block region of custom theme can be captured with '#page-content div.block-region', it should work, but it looks like it uses different CSS selectors. Theme author should try to adjust the theme first, if this will not work for some reason, then I will have a look and we decide how to make the feature more flexible.
          Hide
          Matthew Cannings added a comment - - edited

          Hi Ruslan,
          I have just tried this and generated an error. Hopefully I have everything correct.
          In the theme I have two extra areas defined in the theme config using

              'course' => array(
                  'file' => 'course.php',
                  'regions' => array('side-pre', 'side-post' , 'centre-top', 'centre-bottom'),
                  'defaultregion' => 'side-post'
              ),
          

          in the course.php layout (I have just added lines 1 and 3, the middle line was there before for 2.0+)

              echo '<div id="region-centre-top" class="block-region"><div class="region-content">';
              echo $OUTPUT->blocks_for_region('centre-top');
              echo '</div></div>';
          

          This is creating an area that I can drop items into, but I get the error

          Coding error detected, it must be fixed by a programmer: 
             Trying to reference an unknown block region side-centre-top
          

          also

          Stack Trace:
          * line 860 of /lib/blocklib.php: coding_exception thrown
          * line 962 of /lib/blocklib.php: call to block_manager->check_region_is_known()
          * line 302 of /lib/blocklib.php: call to block_manager->ensure_instances_exist()
          * line 60 of /lib/ajax/blocks.php: call to block_manager->get_blocks_for_region()
          

          It is renaming the section from centre-top to side-centre-top.

          Show
          Matthew Cannings added a comment - - edited Hi Ruslan, I have just tried this and generated an error. Hopefully I have everything correct. In the theme I have two extra areas defined in the theme config using 'course' => array( 'file' => 'course.php', 'regions' => array('side-pre', 'side-post' , 'centre-top', 'centre-bottom'), 'defaultregion' => 'side-post' ), in the course.php layout (I have just added lines 1 and 3, the middle line was there before for 2.0+) echo '<div id= "region-centre-top" class= "block-region" ><div class= "region-content" >'; echo $OUTPUT->blocks_for_region('centre-top'); echo '</div></div>'; This is creating an area that I can drop items into, but I get the error Coding error detected, it must be fixed by a programmer: Trying to reference an unknown block region side-centre-top also Stack Trace: * line 860 of /lib/blocklib.php: coding_exception thrown * line 962 of /lib/blocklib.php: call to block_manager->check_region_is_known() * line 302 of /lib/blocklib.php: call to block_manager->ensure_instances_exist() * line 60 of /lib/ajax/blocks.php: call to block_manager->get_blocks_for_region() It is renaming the section from centre-top to side-centre-top .
          Hide
          Andrew Nicols added a comment - - edited

          Hi Matthew,

          I think if you change the:

          <div id="region-centre-top" class="block-region">
          

          to

          <div id="centre-top" class="block-region">
          

          then I believe that may solve the issue.

          The block drag/drop code has to convert region to side because core themes have historically referred to the side-pre and side-post in the html as region-pre and region-post and this was the only way that we could maintain backwards compatibility for as many themes as possible.

          Show
          Andrew Nicols added a comment - - edited Hi Matthew, I think if you change the: <div id= "region-centre-top" class= "block-region" > to <div id= "centre-top" class= "block-region" > then I believe that may solve the issue. The block drag/drop code has to convert region to side because core themes have historically referred to the side-pre and side-post in the html as region-pre and region-post and this was the only way that we could maintain backwards compatibility for as many themes as possible.
          Hide
          Matthew Cannings added a comment - - edited

          Hi Andrew,
          I can confirm that this has now worked for my theme running Moodle 2.3beta.

          In the theme I have two extra areas defined in the theme config using

              'course' => array(
                  'file' => 'course.php',
                  'regions' => array('side-pre', 'side-post' , 'centre-top', 'centre-bottom'),
                  'defaultregion' => 'side-post'
              ),
          

          in the course.php layout

              echo '<div id="centre-top" class="block-region"><div class="region-content">';
              echo $OUTPUT->blocks_for_region('centre-top');
              echo '</div></div>';
              echo core_renderer::MAIN_CONTENT_TOKEN; 
              echo '<div id="centre-bottom" class="block-region"><div class="region-content">';
              echo $OUTPUT->blocks_for_region('centre-bottom');
              echo '</div></div>'; 
          

          (There is a slight issue but I think this is to do with CSS in my theme so will not post unless others experience it )

          Show
          Matthew Cannings added a comment - - edited Hi Andrew, I can confirm that this has now worked for my theme running Moodle 2.3beta. In the theme I have two extra areas defined in the theme config using 'course' => array( 'file' => 'course.php', 'regions' => array('side-pre', 'side-post' , 'centre-top', 'centre-bottom'), 'defaultregion' => 'side-post' ), in the course.php layout echo '<div id= "centre-top" class= "block-region" ><div class= "region-content" >'; echo $OUTPUT->blocks_for_region('centre-top'); echo '</div></div>'; echo core_renderer::MAIN_CONTENT_TOKEN; echo '<div id= "centre-bottom" class= "block-region" ><div class= "region-content" >'; echo $OUTPUT->blocks_for_region('centre-bottom'); echo '</div></div>'; (There is a slight issue but I think this is to do with CSS in my theme so will not post unless others experience it )
          Hide
          Mary Evans added a comment -

          Have you declared the the new regions in the very top of general.php and frontpage.php like it is for $hassidepre and $hassideport you should have $hascentertop and $hascenterbottom here is how it looks in AArdvark Post-IT

          $haspbarpre = $PAGE->blocks->region_has_content('pbar-pre', $OUTPUT);
          $haspbarpost = $PAGE->blocks->region_has_content('pbar-post', $OUTPUT);
          

          Also you need to add the strings for you new block-regions.
          For example from theme_aardvark_postit.php

          $string['region-pbar-post'] = 'Profile Bar Right';
          $string['region-pbar-pre'] = 'Profile Bar Left';
          
          Show
          Mary Evans added a comment - Have you declared the the new regions in the very top of general.php and frontpage.php like it is for $hassidepre and $hassideport you should have $hascentertop and $hascenterbottom here is how it looks in AArdvark Post-IT $haspbarpre = $PAGE->blocks->region_has_content('pbar-pre', $OUTPUT); $haspbarpost = $PAGE->blocks->region_has_content('pbar-post', $OUTPUT); Also you need to add the strings for you new block-regions. For example from theme_aardvark_postit.php $string['region-pbar-post'] = 'Profile Bar Right'; $string['region-pbar-pre'] = 'Profile Bar Left';
          Hide
          Matthew Cannings added a comment - - edited

          Hi Mary,
          Thanks, those had already been done so not the cause of the problem.

          The problem I was having does appear to be javascript related.
          I can drag things from side-pre and side-post into either centre-top and centre-bottom no problem after the change posted above.

          If I drag something from centre-top and whilst dragging I hover over side-pre or side-post but do not drop as soon as I move away from another block region .side-pre-only or .side-post-only (and also .side-centre-bottom-only) are being added as classes to the body tag even though these regions do have blocks in.

          I suspect the javascript for drag drop does not know how to deal with a block picked up from a region that is not side-pre or side-post

          Show
          Matthew Cannings added a comment - - edited Hi Mary, Thanks, those had already been done so not the cause of the problem. The problem I was having does appear to be javascript related. I can drag things from side-pre and side-post into either centre-top and centre-bottom no problem after the change posted above. If I drag something from centre-top and whilst dragging I hover over side-pre or side-post but do not drop as soon as I move away from another block region .side-pre-only or .side-post-only (and also .side-centre-bottom-only) are being added as classes to the body tag even though these regions do have blocks in. I suspect the javascript for drag drop does not know how to deal with a block picked up from a region that is not side-pre or side-post
          Hide
          Mary Evans added a comment - - edited

          If the block drag/drop code has to convert region to side to maintain backwards compatibility, what is the best way, then, to name all customised block regions?

          side-top | side-top-pre | side-top-post
          side-mid | side-mid-pre | side-mid-post
          side-bot | side-bot-pre | side-bot-post
          

          If this is all we need to do, then this is easy enough to implement in a theme.

          Can I have come clarification on this?

          Thanks

          Show
          Mary Evans added a comment - - edited If the block drag/drop code has to convert region to side to maintain backwards compatibility, what is the best way, then, to name all customised block regions? side-top | side-top-pre | side-top-post side-mid | side-mid-pre | side-mid-post side-bot | side-bot-pre | side-bot-post If this is all we need to do, then this is easy enough to implement in a theme. Can I have come clarification on this? Thanks
          Hide
          Matthew Cannings added a comment -

          Have added a short video showing this in action in case my explanation confused more than explained.

          http://www.youtube.com/watch?v=rUznO_vBd5U

          Show
          Matthew Cannings added a comment - Have added a short video showing this in action in case my explanation confused more than explained. http://www.youtube.com/watch?v=rUznO_vBd5U
          Hide
          Mary Evans added a comment -

          Have you got some CSS with the body tag .blocks-moving ?

          The only ones that are in BASE theme are for side-post-only as this was a problem if you have blocks in that region and wanted to add theme into side-pre. So in this side-post-only situation, when editing is on and you are MOVING blocks the page resets to a full page layout allowing you access to those regions normally hidden.

          You have to envisage this like sliding doors to understand it.

          That's if I understand what YOU are saying.

          Show
          Mary Evans added a comment - Have you got some CSS with the body tag .blocks-moving ? The only ones that are in BASE theme are for side-post-only as this was a problem if you have blocks in that region and wanted to add theme into side-pre. So in this side-post-only situation, when editing is on and you are MOVING blocks the page resets to a full page layout allowing you access to those regions normally hidden. You have to envisage this like sliding doors to understand it. That's if I understand what YOU are saying.
          Hide
          Matthew Cannings added a comment - - edited

          Hi Mary,
          Thanks for your recommendations, for my theme the centre-top and centre-bottom are both within the main content area so the layout issues are taken care of by the css already in the base theme.

          I have been looking at lib/yui/blocks/blocks.js and from the limited amount that I understand of it, it seems as this is written very much with just two block areas in mind.

          From the initial posting the code makes an assumption that the first block-region in the page will be either side-pre or side-post and also assumes that if it is side-post then side-pre must be missing. Really it should check through all block-regions for both pre and post and insert either if they are missing.

          I have done a few changes that seem to work but I am not too great with javascript

          I have replaced line 39

          if (blockregionlist.size() != this.get('regions').length) {
          

          with

              var postexist = false;
              var preexist = false;
              blockregionlist.each(function(blockregionnode) {
                  if (blockregionnode.get('id') === 'region-post'){
          	    postexist = true;
          	} else if (blockregionnode.get('id') === 'region-pre'){
                      preexist = true;
                  }
              })
          			
              if (!(postexist && preexist)){
          

          but this is probably stretching the limits of my javascript. Possibly someone with more knowledge could run with this?
          which seems a better check

          then

              var regionid = this.get_region_id(blockregionlist.item(0));
              if (regionid === 'post') {
                  // pre block is missing, instert it before post
                  blockregion.setAttrs({id : 'region-pre'});
                  blockregionlist.item(0).insert(blockregion, 'before');
                  blockregionlist.unshift(blockregion);
              } else {
                  // post block is missing, instert it after pre
                  blockregion.setAttrs({id : 'region-post'});
                  blockregionlist.item(0).insert(blockregion, 'after');
                  blockregionlist.push(blockregion);
              }
          

          could be replaced with

              if (!preexist) {
                  // pre block is missing, instert it before post
                  blockregion.setAttrs({id : 'region-pre'});
                  blockregionlist.item(0).insert(blockregion, 'before');
                  blockregionlist.unshift(blockregion);
              } 
              if (!postexist) {
                  // post block is missing, instert it after pre
                  blockregion.setAttrs({id : 'region-post'});
                  blockregionlist.item(0).insert(blockregion, 'after');
                  blockregionlist.push(blockregion);
              }
          

          But this is really stretching the limits of my javascript so someone could maybe run with this?

          Show
          Matthew Cannings added a comment - - edited Hi Mary, Thanks for your recommendations, for my theme the centre-top and centre-bottom are both within the main content area so the layout issues are taken care of by the css already in the base theme. I have been looking at lib/yui/blocks/blocks.js and from the limited amount that I understand of it, it seems as this is written very much with just two block areas in mind. From the initial posting the code makes an assumption that the first block-region in the page will be either side-pre or side-post and also assumes that if it is side-post then side-pre must be missing. Really it should check through all block-regions for both pre and post and insert either if they are missing. I have done a few changes that seem to work but I am not too great with javascript I have replaced line 39 if (blockregionlist.size() != this .get('regions').length) { with var postexist = false ; var preexist = false ; blockregionlist.each(function(blockregionnode) { if (blockregionnode.get('id') === 'region-post'){ postexist = true ; } else if (blockregionnode.get('id') === 'region-pre'){ preexist = true ; } }) if (!(postexist && preexist)){ but this is probably stretching the limits of my javascript. Possibly someone with more knowledge could run with this? which seems a better check then var regionid = this .get_region_id(blockregionlist.item(0)); if (regionid === 'post') { // pre block is missing, instert it before post blockregion.setAttrs({id : 'region-pre'}); blockregionlist.item(0).insert(blockregion, 'before'); blockregionlist.unshift(blockregion); } else { // post block is missing, instert it after pre blockregion.setAttrs({id : 'region-post'}); blockregionlist.item(0).insert(blockregion, 'after'); blockregionlist.push(blockregion); } could be replaced with if (!preexist) { // pre block is missing, instert it before post blockregion.setAttrs({id : 'region-pre'}); blockregionlist.item(0).insert(blockregion, 'before'); blockregionlist.unshift(blockregion); } if (!postexist) { // post block is missing, instert it after pre blockregion.setAttrs({id : 'region-post'}); blockregionlist.item(0).insert(blockregion, 'after'); blockregionlist.push(blockregion); } But this is really stretching the limits of my javascript so someone could maybe run with this?
          Hide
          Matthew Cannings added a comment -

          Just reread the code that I posted and that still assumes that region-pre is the first block region in the page during the insert before and after sections. It could possibly be changed to insert both after the content area? but even then it is enforcing the location on a theme creater.

          I can not see this working unless all the block-regions are already on the page even if they have no content and using CSS to hide them and make them visible when required.

          Show
          Matthew Cannings added a comment - Just reread the code that I posted and that still assumes that region-pre is the first block region in the page during the insert before and after sections. It could possibly be changed to insert both after the content area? but even then it is enforcing the location on a theme creater. I can not see this working unless all the block-regions are already on the page even if they have no content and using CSS to hide them and make them visible when required.
          Hide
          Ruslan Kabalin added a comment -

          Matthew: yep, I only had two blocks in mind initially, that is why you observe that wierdness with wrong blocks disappearing.

          I can not see this working unless all the block-regions are already on the page even if they have no content and using CSS to hide them and make them visible when required.

          That is correct, we have MDL-32608 to ensure that regions exists irrespective to the theme output. I do not think we have time to implement this properly before 2.3, but what I will do now is to play with your theme and see what can be done instead.

          Show
          Ruslan Kabalin added a comment - Matthew: yep, I only had two blocks in mind initially, that is why you observe that wierdness with wrong blocks disappearing. I can not see this working unless all the block-regions are already on the page even if they have no content and using CSS to hide them and make them visible when required. That is correct, we have MDL-32608 to ensure that regions exists irrespective to the theme output. I do not think we have time to implement this properly before 2.3, but what I will do now is to play with your theme and see what can be done instead.
          Hide
          Ruslan Kabalin added a comment -

          Matthew, what theme are you using in examples?

          Show
          Ruslan Kabalin added a comment - Matthew, what theme are you using in examples?
          Hide
          Mary Evans added a comment -

          You can d/load my Aardvark Post-IT theme if you like?

          https://github.com/lazydaisy/Moodle-Studio/tree/aardvark_postit

          Show
          Mary Evans added a comment - You can d/load my Aardvark Post-IT theme if you like? https://github.com/lazydaisy/Moodle-Studio/tree/aardvark_postit
          Hide
          Ruslan Kabalin added a comment -

          Added the fix to master that makes blocks dragdrop less theme dependant. Theme maintainers still need to make sure that block regions are wrapped in correct selectors.

          Show
          Ruslan Kabalin added a comment - Added the fix to master that makes blocks dragdrop less theme dependant. Theme maintainers still need to make sure that block regions are wrapped in correct selectors.
          Hide
          Ruslan Kabalin added a comment - - edited

          Mary: Aardvark Post-IT needs attached patch to work correctly.

          Show
          Ruslan Kabalin added a comment - - edited Mary: Aardvark Post-IT needs attached patch to work correctly.
          Hide
          Mary Evans added a comment -

          Matthew, the way side-pre and post is not governed by css, it is governed by the theme's config.php in $THEME->layouts. Here you can declare which regions are going to be present, and which is the default region. The default region is the one that has the ADD BLOCK. This has been changed recently from side-post to side-pre. This was so that the ADD BLOCK would not create problems in a page when editing is on. If you ever noticed the ADD BLOCK would take up space on a page whenever editing was enabled.
          CSS looks after the layout, but the layout is set in the config.php and in the layout/general.php and frontpage.php where the body classes are set.
          So if a page has a side-post-only setting, as soon as editing is on and blocks are being moved the page returns to a normal layout, (all 3 columns in a 3 coll layout) so it is not hiding anything.

          Show
          Mary Evans added a comment - Matthew, the way side-pre and post is not governed by css, it is governed by the theme's config.php in $THEME->layouts. Here you can declare which regions are going to be present, and which is the default region. The default region is the one that has the ADD BLOCK. This has been changed recently from side-post to side-pre. This was so that the ADD BLOCK would not create problems in a page when editing is on. If you ever noticed the ADD BLOCK would take up space on a page whenever editing was enabled. CSS looks after the layout, but the layout is set in the config.php and in the layout/general.php and frontpage.php where the body classes are set. So if a page has a side-post-only setting, as soon as editing is on and blocks are being moved the page returns to a normal layout, (all 3 columns in a 3 coll layout) so it is not hiding anything.
          Hide
          Mary Evans added a comment -

          Try adding the patch and it still does not work!

          Show
          Mary Evans added a comment - Try adding the patch and it still does not work!
          Hide
          Ruslan Kabalin added a comment -

          Do you add both, this bug fix and patch?

          Show
          Ruslan Kabalin added a comment - Do you add both, this bug fix and patch ?
          Hide
          Mary Evans added a comment -

          Sorry...I missed the BUG patch.

          I thought you said originally that the block.js was nothing to do with this problem?
          I'll go see what happens when it is added.
          Thanks

          Show
          Mary Evans added a comment - Sorry...I missed the BUG patch. I thought you said originally that the block.js was nothing to do with this problem? I'll go see what happens when it is added. Thanks
          Hide
          Mary Evans added a comment -

          For some reason the AJAX has now stopped working so I am about to restart my localhost WAMP server and login again.

          Show
          Mary Evans added a comment - For some reason the AJAX has now stopped working so I am about to restart my localhost WAMP server and login again.
          Hide
          Mary Evans added a comment -

          It seems to have reverted back to the old way of Moving BLOCKS

          Show
          Mary Evans added a comment - It seems to have reverted back to the old way of Moving BLOCKS
          Hide
          Ruslan Kabalin added a comment -

          If you disable JS or have JS errors, you will the old-way will be used.

          Show
          Ruslan Kabalin added a comment - If you disable JS or have JS errors, you will the old-way will be used.
          Hide
          Mary Evans added a comment -

          Adding block-region and region-content

          <?php if ($haspbarpost) { ?>
              <div class="block-region">
                  <div class="region-content">
                      <?php echo $OUTPUT->blocks_for_region('pbar-post');?>
                  </div>
              </div>
          <?php } ?>
          

          works but gives the following errors...

          Coding error detected, it must be fixed by a programmer: Trying to reference an unknown block region yui______
          URL: /
          Debug info: Error code: codingerror
          Stack trace:
          
          * line 860 of \lib\blocklib.php: coding_exception thrown
          * line 962 of \lib\blocklib.php: call to block_manager->check_region_is_known()
          * line 302 of \lib\blocklib.php: call to block_manager->ensure_instances_exist()
          * line 60 of \lib\ajax\blocks.php: call to block_manager->get_blocks_for_region()
          
          
          Show
          Mary Evans added a comment - Adding block-region and region-content <?php if ($haspbarpost) { ?> <div class= "block-region" > <div class= "region-content" > <?php echo $OUTPUT->blocks_for_region('pbar-post');?> </div> </div> <?php } ?> works but gives the following errors... Coding error detected, it must be fixed by a programmer: Trying to reference an unknown block region yui______ URL: / Debug info: Error code: codingerror Stack trace: * line 860 of \lib\blocklib.php: coding_exception thrown * line 962 of \lib\blocklib.php: call to block_manager->check_region_is_known() * line 302 of \lib\blocklib.php: call to block_manager->ensure_instances_exist() * line 60 of \lib\ajax\blocks.php: call to block_manager->get_blocks_for_region()
          Hide
          Mary Evans added a comment -

          @Ruslan

          It seems I missed the patch you added, I thought you were talking about the comment from Andrew.

          Show
          Mary Evans added a comment - @Ruslan It seems I missed the patch you added, I thought you were talking about the comment from Andrew.
          Hide
          Mary Evans added a comment -

          Well that fixed it!
          Can the change to the block.js be added to Moodle 2.3?
          It would be great if it could.

          Thanks!

          Show
          Mary Evans added a comment - Well that fixed it! Can the change to the block.js be added to Moodle 2.3? It would be great if it could. Thanks!
          Hide
          Ruslan Kabalin added a comment -

          Works for reporter, please someone peer review it.

          Show
          Ruslan Kabalin added a comment - Works for reporter, please someone peer review it.
          Hide
          Andrew Nicols added a comment -

          The code looks good to me.
          Could you just add a comment to the code though explaining why we only act upon blocks whose ID matches region-pre or region-post?

          Show
          Andrew Nicols added a comment - The code looks good to me. Could you just add a comment to the code though explaining why we only act upon blocks whose ID matches region-pre or region-post?
          Hide
          Ruslan Kabalin added a comment -

          Addressed Andrew's comments. Ready for integration.

          Show
          Ruslan Kabalin added a comment - Addressed Andrew's comments. Ready for integration.
          Hide
          Matthew Cannings added a comment -

          Hi All,
          I have just added the patch on 2.3rc1 20120624 and everything is now working in the theme I have.

          Hi Ruslan,
          The theme I use is a modified version of Leatherbound, unfortunately it contains lots of code that would not work outside of our Moodle instance (custom headers, footer, navigation etc...). I am looking to remove most of that and release it but it needs a lot of testing before it is ready For the purpose of this problem the only difference is minor changes to the layout course.php and config.php to add the centre-top and centre-bottom either side of core_renderer::MAIN_CONTENT_TOKEN

          Show
          Matthew Cannings added a comment - Hi All, I have just added the patch on 2.3rc1 20120624 and everything is now working in the theme I have. Hi Ruslan, The theme I use is a modified version of Leatherbound, unfortunately it contains lots of code that would not work outside of our Moodle instance (custom headers, footer, navigation etc...). I am looking to remove most of that and release it but it needs a lot of testing before it is ready For the purpose of this problem the only difference is minor changes to the layout course.php and config.php to add the centre-top and centre-bottom either side of core_renderer::MAIN_CONTENT_TOKEN
          Hide
          Ruslan Kabalin added a comment -

          Added instructions.

          Show
          Ruslan Kabalin added a comment - Added instructions.
          Hide
          Martin Dougiamas added a comment -

          Since we're so close to 2.3 now we might have to land this afterwards for 2.3.1

          Show
          Martin Dougiamas added a comment - Since we're so close to 2.3 now we might have to land this afterwards for 2.3.1
          Hide
          Mary Evans added a comment -

          @ Matthew
          core_renderer::MAIN_CONTENT_TOKEN has been replaced by %OUTPUT->main_content()in Moodle 2.2 so you would be advised to amend your current theme to reflect this change to avoid conflicts in Moodle 2.3

          Show
          Mary Evans added a comment - @ Matthew core_renderer::MAIN_CONTENT_TOKEN has been replaced by %OUTPUT->main_content()in Moodle 2.2 so you would be advised to amend your current theme to reflect this change to avoid conflicts in Moodle 2.3
          Hide
          Mary Evans added a comment -

          @Martin I suppose at this late stage this in unavoidable, so better in Moodle 2.3.1 than not at all!

          Thanks

          Show
          Mary Evans added a comment - @Martin I suppose at this late stage this in unavoidable, so better in Moodle 2.3.1 than not at all! Thanks
          Hide
          Mary Evans added a comment - - edited

          @Ruslan

          There are other issues with Aardvark Post-IT after blocks have been moved into the custom profilebar in the header. If the remaining blocks on the page are docked the page does not resize correctly, as it sees the hidden blocks in the profile bar as being on the page still. That is the reason I left off the block-region class originally. Hopefully I can fix this problem in the theme itself as it is nothing to do with Moodle at this stage. At least I think so! I think it has something to do with the $haspbarpre and $haspbarpost and now needs $showpbarpre and $showpbarpost too.

          Thanks for all your work with this issue.

          Show
          Mary Evans added a comment - - edited @Ruslan There are other issues with Aardvark Post-IT after blocks have been moved into the custom profilebar in the header. If the remaining blocks on the page are docked the page does not resize correctly, as it sees the hidden blocks in the profile bar as being on the page still. That is the reason I left off the block-region class originally. Hopefully I can fix this problem in the theme itself as it is nothing to do with Moodle at this stage. At least I think so! I think it has something to do with the $haspbarpre and $haspbarpost and now needs $showpbarpre and $showpbarpost too. Thanks for all your work with this issue.
          Hide
          Ruslan Kabalin added a comment -

          @Mary NP, my pleasure, hopefully MDL-32608 will make blocks drag-drop implementation even less theme-dependant.

          Show
          Ruslan Kabalin added a comment - @Mary NP, my pleasure, hopefully MDL-32608 will make blocks drag-drop implementation even less theme-dependant.
          Hide
          Dan Poltawski added a comment -

          Hi Ruslan,

          This is conflicting with master, please can you sort out the conflicts (and also should clenly cherry-pick with 23_STABLE).

          Show
          Dan Poltawski added a comment - Hi Ruslan, This is conflicting with master, please can you sort out the conflicts (and also should clenly cherry-pick with 23_STABLE).
          Hide
          CiBoT added a comment -

          Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

          Show
          CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
          Hide
          Mary Evans added a comment -

          This should not be happening! We need this bug fix.
          I was relying on this getting integrated into Moodle 2.3.1

          What a disappointment!

          Show
          Mary Evans added a comment - This should not be happening! We need this bug fix. I was relying on this getting integrated into Moodle 2.3.1 What a disappointment!
          Hide
          Jean-Michel Vedrine added a comment -

          This is very infortunate for me too, as I am using a modified version of AArvark Postit on my production website.
          I absolutely need this bugfix. I saw it was postponed to 2.3.2. Too bad !!
          Please can this be sorted out ASAP ?

          Show
          Jean-Michel Vedrine added a comment - This is very infortunate for me too, as I am using a modified version of AArvark Postit on my production website. I absolutely need this bugfix. I saw it was postponed to 2.3.2. Too bad !! Please can this be sorted out ASAP ?
          Hide
          Ruslan Kabalin added a comment -

          Has been reviewed already, branch has been updated, ready for integration.

          Show
          Ruslan Kabalin added a comment - Has been reviewed already, branch has been updated, ready for integration.
          Hide
          Dan Poltawski added a comment -

          Thanks Ruslan, I've integrated this now.

          Please don't forget about 23 branch. In this case it cherry picked cleanly.

          Show
          Dan Poltawski added a comment - Thanks Ruslan, I've integrated this now. Please don't forget about 23 branch. In this case it cherry picked cleanly.
          Hide
          Mary Evans added a comment - - edited

          @Jean-Michel Vedrine

          Once this fix has been tested and passed, it will (I hope) be available in the next update for Moodle 2.3.1 which could be this week.

          You will also need the updated Aardvark Post-IT which you can download from my GITHUB

          https://github.com/lazydaisy/Moodle-Studio/zipball/aardvark_postit

          Thanks
          Mary

          Show
          Mary Evans added a comment - - edited @Jean-Michel Vedrine Once this fix has been tested and passed, it will (I hope) be available in the next update for Moodle 2.3.1 which could be this week. You will also need the updated Aardvark Post-IT which you can download from my GITHUB https://github.com/lazydaisy/Moodle-Studio/zipball/aardvark_postit Thanks Mary
          Hide
          Jason Fowler added a comment -

          I can't seem to get the theme in order to test it, could you please attach the zipped theme for me to test with?

          Show
          Jason Fowler added a comment - I can't seem to get the theme in order to test it, could you please attach the zipped theme for me to test with?
          Hide
          Dan Poltawski added a comment -
          Show
          Dan Poltawski added a comment - Jason, search the plugins directory: http://moodle.org/plugins/view.php?plugin=theme_aardvark_postit
          Hide
          Jason Fowler added a comment -

          Did that Dan, but the instructions are "install the theme with the patch - I am not sure if the 2.2 version on the plugins directory has the patch for 2.3

          Show
          Jason Fowler added a comment - Did that Dan, but the instructions are "install the theme with the patch - I am not sure if the 2.2 version on the plugins directory has the patch for 2.3
          Hide
          Dan Poltawski added a comment -

          Sigh.

          Rather than slowing down the whole process, you could try it. (Just like Ruslan managed to do when writing the fix.)

          Show
          Dan Poltawski added a comment - Sigh. Rather than slowing down the whole process, you could try it. (Just like Ruslan managed to do when writing the fix.)
          Hide
          Ruslan Kabalin added a comment - - edited

          @Jason: the updated Aardvark Post-IT is just 4 comments above. You may also use the older version of the theme and the patch I suggested in this issue.

          Show
          Ruslan Kabalin added a comment - - edited @Jason: the updated Aardvark Post-IT is just 4 comments above . You may also use the older version of the theme and the patch I suggested in this issue.
          Hide
          Mary Evans added a comment -

          I had to delete the latest version as everyone was getting errors because of some bad coding! The attached is the original with the patch.

          Show
          Mary Evans added a comment - I had to delete the latest version as everyone was getting errors because of some bad coding! The attached is the original with the patch.
          Hide
          Mary Evans added a comment - - edited

          However I have just realised that core_renderer::MAIN_CONTENT_TOKEN needs changing to $OUTPUT->main_content() in layout/default.php

          Show
          Mary Evans added a comment - - edited However I have just realised that core_renderer::MAIN_CONTENT_TOKEN needs changing to $OUTPUT->main_content() in layout/default.php
          Hide
          Mary Evans added a comment -

          I don;t know how you are getting on today with testing this Jason, but I am wishing I never created the theme! So far I have done everything wrong. So the zip file I added is wrong too...so I have deleted it. Although the theme I was using earlier worked great, I just messed it up trying to restyle it for Moodle 2.3.

          I think it is a case of more haste less speed...so I'll go and rest in a darkened room and try again later.

          Show
          Mary Evans added a comment - I don;t know how you are getting on today with testing this Jason, but I am wishing I never created the theme! So far I have done everything wrong. So the zip file I added is wrong too...so I have deleted it. Although the theme I was using earlier worked great, I just messed it up trying to restyle it for Moodle 2.3. I think it is a case of more haste less speed...so I'll go and rest in a darkened room and try again later.
          Hide
          Jason Fowler added a comment -

          I hadn't started, as I left before you provided the download. What do we do now? do you have something that works that I can test?

          Show
          Jason Fowler added a comment - I hadn't started, as I left before you provided the download. What do we do now? do you have something that works that I can test?
          Hide
          Dan Poltawski added a comment -

          Jason: I have downloaded the theme and applied the patch for you.

          https://dl.dropbox.com/u/5403781/theme_for_jason.zip

          Show
          Dan Poltawski added a comment - Jason: I have downloaded the theme and applied the patch for you. https://dl.dropbox.com/u/5403781/theme_for_jason.zip
          Hide
          Jason Fowler added a comment -

          Thanks Dan

          Show
          Jason Fowler added a comment - Thanks Dan
          Hide
          Jason Fowler added a comment -

          That theme does not seem to have the custom region at the top that is mentioned in the testing instructions.

          Show
          Jason Fowler added a comment - That theme does not seem to have the custom region at the top that is mentioned in the testing instructions.
          Hide
          Dan Poltawski added a comment -

          Jason, please test the regression testing steps and ignore the advark then.

          Show
          Dan Poltawski added a comment - Jason, please test the regression testing steps and ignore the advark then.
          Hide
          Jason Fowler added a comment -

          existing functionality works fine, couldn't test the extra region stuff though

          Show
          Jason Fowler added a comment - existing functionality works fine, couldn't test the extra region stuff though
          Hide
          Mary Evans added a comment -

          I can confirm that the custom block regions work providing they are coded correctly with div id that matched the block region and that it is classed as a block-region, if that is any consolation? I've added the updated theme I am currently working on and that works great. I'm just in the middle of fixing some CSS issues, but that's nothing to do with the custom blocks.

          Thanks

          Show
          Mary Evans added a comment - I can confirm that the custom block regions work providing they are coded correctly with div id that matched the block region and that it is classed as a block-region, if that is any consolation? I've added the updated theme I am currently working on and that works great. I'm just in the middle of fixing some CSS issues, but that's nothing to do with the custom blocks. Thanks
          Hide
          Dan Poltawski added a comment -

          Congratulations!

          You've made it into the weekly release!

          Thanks for your contribution - here are some random drummers to keep you inspired for the next week!
          http://www.youtube.com/watch?v=_QhpHUmVCmY

          Show
          Dan Poltawski added a comment - Congratulations! You've made it into the weekly release! Thanks for your contribution - here are some random drummers to keep you inspired for the next week! http://www.youtube.com/watch?v=_QhpHUmVCmY
          Hide
          Ruslan Kabalin added a comment -

          @Dan: Not that random really

          Show
          Ruslan Kabalin added a comment - @Dan: Not that random really
          Hide
          Paul Nicholls added a comment -

          Woops, this isn't the one I meant to assign to me! (Too many things open, too little coffee...)

          Show
          Paul Nicholls added a comment - Woops, this isn't the one I meant to assign to me! (Too many things open, too little coffee...)
          Hide
          Mary Evans added a comment -

          Show
          Mary Evans added a comment -
          Hide
          Elizabeth Dalton added a comment -

          I downloaded the version of Aardvark Post-It attached at the top of this ticket onto a Moodle 2.3.3 server, but blocks still vanish if they get moved to a custom region. Do I need the patch as well, even if I'm on 2.3.3?

          Show
          Elizabeth Dalton added a comment - I downloaded the version of Aardvark Post-It attached at the top of this ticket onto a Moodle 2.3.3 server, but blocks still vanish if they get moved to a custom region. Do I need the patch as well, even if I'm on 2.3.3?
          Hide
          Mary Evans added a comment -

          Elizabeth,

          The Postit theme listed here is not right. I should heave deleted it.
          The correct one is in MDL-34496.

          It should work in Moodle 2.3.2, but not with Dock enabled. You may be able to use the Drag n Drop but even that is not working correctly for this theme at least.

          If you experience any problem you can leave a comment in MDL-34496

          Thanks
          Mary

          Show
          Mary Evans added a comment - Elizabeth, The Postit theme listed here is not right. I should heave deleted it. The correct one is in MDL-34496 . It should work in Moodle 2.3.2, but not with Dock enabled. You may be able to use the Drag n Drop but even that is not working correctly for this theme at least. If you experience any problem you can leave a comment in MDL-34496 Thanks Mary

            People

            • Votes:
              7 Vote for this issue
              Watchers:
              14 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: