Moodle
  1. Moodle
  2. MDL-30010

FIX all CORE themes which have pagelayout problems when moving blocks

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 2.0.3, 2.1.2, 2.2
    • Fix Version/s: 2.0.6, 2.1.3
    • Component/s: Themes
    • Labels:
      None
    • Testing Instructions:
      Hide
      • go to a side-post-only course
      • turn editing on
      • dock one block, dock all blocks
      • undock all blocks
      • turn moving blocks on
      • dock one block, dock all blocks
      • undock all blocks
      Show
      go to a side-post-only course turn editing on dock one block, dock all blocks undock all blocks turn moving blocks on dock one block, dock all blocks undock all blocks
    • Workaround:
      Hide

      We need to work out a strategy how this can best be fixed!

      Show
      We need to work out a strategy how this can best be fixed!
    • Affected Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      wip-MDL-30010
    • Rank:
      19554

      Description

      We have 18 core themes + base and canvas. From these 20 themes

      • 3 blog-style themes are fixed (nonzero, overlay, sky_high) and work as they should
      • 5 only have a right column (arialist, binarius, brick fusion, nimble) - so there is no side-post-only problem as all blocks stay at side-post when moving them.
      • 8 have major bugs (afterburner, anomaly, formal_white, leatherbound [very strange!], magazine [very strange!], serenity, splash, standard)
      • 4 work halfway fine (base, canvas, boxxie, formfactor) [because they do not allow the docking of blocks!]

      [I have attached the results as .xls file!]

      We have a big amount of bugs and strange behaviours.
      We are sure that these are not only theme issues.
      We will need the help of the core developers to FIX these issues.

      1. blocklib.php
        80 kB
        Dietmar Wagner
      2. core_themes_side_post_only_behaviour.xls
        33 kB
        Mary Evans

        Issue Links

          Activity

          Hide
          Mary Evans added a comment -

          @Dietmar

          I have added you as a watcher here so that we can get started on this mammoth task!
          I hope you had a relaxing holiday!

          Cheers
          Mary

          Show
          Mary Evans added a comment - @Dietmar I have added you as a watcher here so that we can get started on this mammoth task! I hope you had a relaxing holiday! Cheers Mary
          Hide
          Mary Evans added a comment -

          @Sam
          I have added you as a watcher Sam as Deitmar and I are going to need some advice with this project.
          Cheers
          Mary

          Show
          Mary Evans added a comment - @Sam I have added you as a watcher Sam as Deitmar and I are going to need some advice with this project. Cheers Mary
          Hide
          Dietmar Wagner added a comment -

          @Mary
          Thanks setting up this tracker issue. I`m still in holiday. Will be back in two days.
          Cheers
          Dietmar

          Show
          Dietmar Wagner added a comment - @Mary Thanks setting up this tracker issue. I`m still in holiday. Will be back in two days. Cheers Dietmar
          Hide
          Dietmar Wagner added a comment -

          Hi Mary,

          I think we should at first find out which of these issues are theme issues and which of them can be repaired by changing moodle core.

          Some thoughts to this:

          If you take e.g. leatherbound - one of our core themes with a very strange behaviour in .side-post-only situations - and disable docking ($THEME->enable_dock = false; in config.php) you will see that now all things (editing and moving blocks) work fine especially if we use the patch form MDL 29674 in lib/blocklib.php.

          On the other hand: If you enable docking in themes that work nearly fine like base or formfactor ($THEME->enable_dock = true; in config.php) you will see some strange behaviours while going through the test instructions as described at the top of this tracker issue.

          That means that the body classes .has_dock and/or .has_dock_left_vertical (and so on) interfer with the body classes .side-post-only, .edting and .blocks-moving in a very disadvantageous way. For me this is not a theme issue!?

          Where can we find substantial theme issues?

          • in the config.php as some themes are using ('base'), others ('canvas', 'base'), and again others ('base', 'canvas') as parent themes, what somehow makes a big difference!
          • in the first 30 lines in general.php / default.php of the all core themes. We can find some significant but not crucial differences here!
          • in the pagelayouts (very unlikely)!?
          • ...

          In my opinion we should - with Sam's assistance - at first "teach" base and/or canvas to work properly when docking is enabled. Anything else should be done afterwards.

          Dietmar

          One question:

          All pagelayout.css include the rule
          .has_dock.side-post-only .page-middle #region-main-box #region-post-box #region-main-wrap #region-main

          { margin-left: 210px; }

          What is that good for?

          Show
          Dietmar Wagner added a comment - Hi Mary, I think we should at first find out which of these issues are theme issues and which of them can be repaired by changing moodle core. Some thoughts to this: If you take e.g. leatherbound - one of our core themes with a very strange behaviour in .side-post-only situations - and disable docking ($THEME->enable_dock = false; in config.php) you will see that now all things (editing and moving blocks) work fine especially if we use the patch form MDL 29674 in lib/blocklib.php. On the other hand: If you enable docking in themes that work nearly fine like base or formfactor ($THEME->enable_dock = true; in config.php) you will see some strange behaviours while going through the test instructions as described at the top of this tracker issue. That means that the body classes .has_dock and/or .has_dock_left_vertical (and so on) interfer with the body classes .side-post-only, .edting and .blocks-moving in a very disadvantageous way. For me this is not a theme issue!? Where can we find substantial theme issues? in the config.php as some themes are using ('base'), others ('canvas', 'base'), and again others ('base', 'canvas') as parent themes, what somehow makes a big difference! in the first 30 lines in general.php / default.php of the all core themes. We can find some significant but not crucial differences here! in the pagelayouts (very unlikely)!? ... In my opinion we should - with Sam's assistance - at first "teach" base and/or canvas to work properly when docking is enabled. Anything else should be done afterwards. Dietmar One question: All pagelayout.css include the rule .has_dock.side-post-only .page-middle #region-main-box #region-post-box #region-main-wrap #region-main { margin-left: 210px; } What is that good for?
          Hide
          Mary Evans added a comment -

          Hi Deitmar, Welcome back!

          You have made some very interesting observations. I totally agree that we must get Base and Canvas working correctly before we can work on the other themes.

          Yesterday evening I came across a very odd situation while I was fixing some tracker issues in the Anomaly theme. I was darting between themes, using "Theme change by URL" to see how each theme rendered a certain page. I eventually ended up comparing Base theme, only to find that the page I was viewing, {side-post-only) rendered itself in Base theme as 3 columns, but there was no content in side-pre! There was no body class for this in the HTML. How ODD is that? Mind you I have yet to replicate it.

          If we are to get Base & Canvas working properly then we need to concentrate on PAGELAYOUT.CSS don't we, since that is where it all hinges?

          Cheers
          Mary

          Show
          Mary Evans added a comment - Hi Deitmar, Welcome back! You have made some very interesting observations. I totally agree that we must get Base and Canvas working correctly before we can work on the other themes. Yesterday evening I came across a very odd situation while I was fixing some tracker issues in the Anomaly theme. I was darting between themes, using "Theme change by URL" to see how each theme rendered a certain page. I eventually ended up comparing Base theme, only to find that the page I was viewing, {side-post-only) rendered itself in Base theme as 3 columns, but there was no content in side-pre! There was no body class for this in the HTML. How ODD is that? Mind you I have yet to replicate it. If we are to get Base & Canvas working properly then we need to concentrate on PAGELAYOUT.CSS don't we, since that is where it all hinges? Cheers Mary
          Hide
          Dietmar Wagner added a comment -

          Hi Mary,

          @Anomaly/Base: If you have a .side-post-only situation and turn editing on, Base opens a gap at side-pre without any content. This behaviour is okay as it is intended from the developers: "If editing is on, we need all the block regions visible, for the move blocks UI." [lib/blocklib.php at about line 354] . If you apply the first patch from MDL-29674 to your blocklib.php the gap only appears if you reallay want to move a block (what's more logical!!!)

          The problem is that most of the 3-column-themes "forget" this intention when docking is enabled.

          @Base/Canvas: Sure, we can force Base and Canvas to render properly by additional rules in pagelayout.css. But I fear we have to find a set of rules for all combinations of body classes. But by this means we will run into a maintainability problem, aren't we? The better way for me is to have at first a closer look at a l l the code that generates the body class .has_dock. And therefor we need the help of a core developer.

          I hope we can gain Sam's attention. Sam?

          Cheers
          Dietmar

          Show
          Dietmar Wagner added a comment - Hi Mary, @Anomaly/Base: If you have a .side-post-only situation and turn editing on, Base opens a gap at side-pre without any content. This behaviour is okay as it is intended from the developers: "If editing is on, we need all the block regions visible, for the move blocks UI." [lib/blocklib.php at about line 354] . If you apply the first patch from MDL-29674 to your blocklib.php the gap only appears if you reallay want to move a block (what's more logical!!!) The problem is that most of the 3-column-themes "forget" this intention when docking is enabled. @Base/Canvas: Sure, we can force Base and Canvas to render properly by additional rules in pagelayout.css. But I fear we have to find a set of rules for all combinations of body classes. But by this means we will run into a maintainability problem, aren't we? The better way for me is to have at first a closer look at a l l the code that generates the body class .has_dock. And therefor we need the help of a core developer. I hope we can gain Sam's attention. Sam? Cheers Dietmar
          Hide
          Dietmar Wagner added a comment -

          F I X E D !!!

          Hi Mary,

          I think I succeeded in fixing all core theme .side-post-only issues.

          What I did:

          1) I applied Mark Nielsen's patch from MDL-29674 to the file lib/blocklib.php. All my trials to succeed without this patch were unsuccessfull!!!
          [I tried to make use of this patch by adding a renderers.php to a theme - what doesn't really make much sense - but I failed!?]
          Thus we ensure that we have a 3 column layout just in the moment when we want to move a block.

          --> core modification !

          2) I added a package of CSS rules to only 3 pagelayouts for the body class combination .blocks-moving.side-post-only (Mary, you know what I mean!).
          Affected themes:
          afterburner -> afterburner-layout.css
          base -> pagelayout.css (to provide all themes that have theme base as parent theme and do not exclude the pagelayout.css with the additional rules)
          magazine -> layout.css

          --> theme modifications !

          3) I went through all themes with our test instructions (primarily with FF7; randomly with other browsers).

          The result:

          afterburner ok
          anomaly ok
          arialist ok (only side post)
          base ok (no docking)
          binarius ok (only side post)
          boxxie ok (no docking)
          brick ok (only side post)
          canvas ok (no docking)
          formal_white ok
          formfactor ok (no docking)
          fusion ok (only side post)
          leatherbound ok
          magazine ok
          nimble ok (only side post)
          nonzero ok (blog, already fixed by M/D)
          overlay ok (blog, already fixed by M/D)
          serenity ok
          sky_high ok (blog, already fixed by M/D)
          splash ok
          standard ok

          In the attachement you can find the m o d i f i e d

          • blocklib.php
          • theme afterburner
          • theme base
          • theme magazine
            as a zip-file.

          I would be glad if you could find some time for testing all this... and I hope the modifications work properly for you too.

          Cheers
          Dietmar

          Show
          Dietmar Wagner added a comment - F I X E D !!! Hi Mary, I think I succeeded in fixing all core theme .side-post-only issues. What I did: 1) I applied Mark Nielsen's patch from MDL-29674 to the file lib/blocklib.php. All my trials to succeed without this patch were unsuccessfull!!! [I tried to make use of this patch by adding a renderers.php to a theme - what doesn't really make much sense - but I failed!?] Thus we ensure that we have a 3 column layout just in the moment when we want to move a block. --> core modification ! 2) I added a package of CSS rules to only 3 pagelayouts for the body class combination .blocks-moving.side-post-only (Mary, you know what I mean!). Affected themes: afterburner -> afterburner-layout.css base -> pagelayout.css (to provide all themes that have theme base as parent theme and do not exclude the pagelayout.css with the additional rules) magazine -> layout.css --> theme modifications ! 3) I went through all themes with our test instructions (primarily with FF7; randomly with other browsers). The result: afterburner ok anomaly ok arialist ok (only side post) base ok (no docking) binarius ok (only side post) boxxie ok (no docking) brick ok (only side post) canvas ok (no docking) formal_white ok formfactor ok (no docking) fusion ok (only side post) leatherbound ok magazine ok nimble ok (only side post) nonzero ok (blog, already fixed by M/D) overlay ok (blog, already fixed by M/D) serenity ok sky_high ok (blog, already fixed by M/D) splash ok standard ok In the attachement you can find the m o d i f i e d blocklib.php theme afterburner theme base theme magazine as a zip-file. I would be glad if you could find some time for testing all this... and I hope the modifications work properly for you too. Cheers Dietmar
          Hide
          Mary Evans added a comment -

          How very interesting, that you should find those three themes all different!
          Well they are arn't they!

          Base = Holy Grail (pixel-width: http://matthewjamestaylor.com/blog/ultimate-3-column-holy-grail-pixels.htm)
          Magazine = Holy Grail (percentage: http://matthewjamestaylor.com/blog/perfect-3-column.htm )
          Afterburner = Holy Grail (no-quirks mode: http://matthewjamestaylor.com/blog/holy-grail-no-quirks-mode.htm)

          I love it!

          In actual fact, of all of these layouts, I much prefer the percentage layout as this is the one I used in my Cafelite 2.0 theme last year. I had not realised that Magazine used this same layout until only recently.

          So do you want me to start work on this and get it committed this weekend?

          Thanks
          Mary

          Show
          Mary Evans added a comment - How very interesting, that you should find those three themes all different! Well they are arn't they! Base = Holy Grail (pixel-width: http://matthewjamestaylor.com/blog/ultimate-3-column-holy-grail-pixels.htm ) Magazine = Holy Grail (percentage: http://matthewjamestaylor.com/blog/perfect-3-column.htm ) Afterburner = Holy Grail (no-quirks mode: http://matthewjamestaylor.com/blog/holy-grail-no-quirks-mode.htm ) I love it! In actual fact, of all of these layouts, I much prefer the percentage layout as this is the one I used in my Cafelite 2.0 theme last year. I had not realised that Magazine used this same layout until only recently. So do you want me to start work on this and get it committed this weekend? Thanks Mary
          Hide
          Mary Evans added a comment - - edited

          Hi Deitmar,
          I am getting an ERROR when trying to MOVE blocks. This is while testing Afterburner in FireFox 7.0.1

          Notice: Undefined property: block_move_target::$blockinstanceid in C:\wamp\www\moodle\lib\blocklib.php on line 462
          Call Stack
          #   Time    Memory  Function    Location
          1   0.0028  45085096    {main}( )   ..\index.php:0
          2   0.6290  74660168    bootstrap_renderer->header( )   ..\index.php:91
          3   0.6290  74660424    bootstrap_renderer->__call( )   ..\setuplib.php:0
          4   0.6445  74827256    call_user_func_array ( )    ..\setuplib.php:1296
          5   0.6445  74827496    core_renderer->header( )    ..\setuplib.php:0
          6   0.6577  75076056    core_renderer->render_page_layout( )    ..\outputrenderers.php:612
          7   0.6582  75168168    include( 'C:\wamp\www\moodle\theme\afterburner\layout\default.php' )    ..\outputrenderers.php:654
          8   1.4480  106844688   block_manager->region_completely_docked( )  ..\default.php:10
          
          Show
          Mary Evans added a comment - - edited Hi Deitmar, I am getting an ERROR when trying to MOVE blocks. This is while testing Afterburner in FireFox 7.0.1 Notice: Undefined property: block_move_target::$blockinstanceid in C:\wamp\www\moodle\lib\blocklib.php on line 462 Call Stack # Time Memory Function Location 1 0.0028 45085096 {main}( ) ..\index.php:0 2 0.6290 74660168 bootstrap_renderer->header( ) ..\index.php:91 3 0.6290 74660424 bootstrap_renderer->__call( ) ..\setuplib.php:0 4 0.6445 74827256 call_user_func_array ( ) ..\setuplib.php:1296 5 0.6445 74827496 core_renderer->header( ) ..\setuplib.php:0 6 0.6577 75076056 core_renderer->render_page_layout( ) ..\outputrenderers.php:612 7 0.6582 75168168 include( 'C:\wamp\www\moodle\theme\afterburner\layout\default.php' ) ..\outputrenderers.php:654 8 1.4480 106844688 block_manager->region_completely_docked( ) ..\default.php:10
          Hide
          Dietmar Wagner added a comment -

          Sorry Mary,
          this error was obviously caused by Mark's second patch for the blocklib.php.
          Please try again with the one attached above. Hope this will work.

          @Holy Grail: Wow, I didn't realize that these three themes have complete different structures.

          I too prefer percentage layouts. If you build your pagelayouts strongly with MJT's presets even blog layouts work properly without any extra css rules. By the way: I learned very much about Themes 2.0 by studying your theme cafelite.

          Cheers
          Dietmar

          Show
          Dietmar Wagner added a comment - Sorry Mary, this error was obviously caused by Mark's second patch for the blocklib.php. Please try again with the one attached above. Hope this will work. @Holy Grail: Wow, I didn't realize that these three themes have complete different structures. I too prefer percentage layouts. If you build your pagelayouts strongly with MJT's presets even blog layouts work properly without any extra css rules. By the way: I learned very much about Themes 2.0 by studying your theme cafelite. Cheers Dietmar
          Hide
          Mary Evans added a comment -

          I find that Moodle is all about trial and error, and working so closely with themes, and more recently for me, course formats too, it make one more aware of what Moodle is, its strengths and capabilities and also it weaknesses too.

          I'll try the new file you added and see how I get on..thanks for that.

          As for the cafelite theme ...I may revamp it and add some new features, now that I know how to!

          Cheers
          Mary

          Show
          Mary Evans added a comment - I find that Moodle is all about trial and error, and working so closely with themes, and more recently for me, course formats too, it make one more aware of what Moodle is, its strengths and capabilities and also it weaknesses too. I'll try the new file you added and see how I get on..thanks for that. As for the cafelite theme ...I may revamp it and add some new features, now that I know how to! Cheers Mary
          Hide
          Mary Evans added a comment -

          It seems to be working fine! I'm just ploughing thru the themes!

          Show
          Mary Evans added a comment - It seems to be working fine! I'm just ploughing thru the themes!
          Hide
          Mary Evans added a comment -

          It WORKS!!!

          I tested the 8 you mentioned above, as there was not point i checking the others as the three we fixed and the others are 2 column themes anyway. So Afterburner, Anomaly, FormFactor, Leatherbound, Magazine, Serenity, Splash, & Standard.all work the same. So I think we can safely say we are on the winning side. Just one more hurdle to go.

          With luck on our side I can submit this for Integration this weekend and provided there are no glitches it might get PULLED into next weeks build! Lets us hope so at least.

          Thanks for all your help with this Dietmar...it is much appreciated.
          Cheers
          Mary

          Show
          Mary Evans added a comment - It WORKS!!! I tested the 8 you mentioned above, as there was not point i checking the others as the three we fixed and the others are 2 column themes anyway. So Afterburner, Anomaly, FormFactor, Leatherbound, Magazine, Serenity, Splash, & Standard.all work the same. So I think we can safely say we are on the winning side. Just one more hurdle to go. With luck on our side I can submit this for Integration this weekend and provided there are no glitches it might get PULLED into next weeks build! Lets us hope so at least. Thanks for all your help with this Dietmar...it is much appreciated. Cheers Mary
          Hide
          Dietmar Wagner added a comment -

          Hi Mary,

          the pleasure is all mine! I'm glad I could help and I will help further on if my help is desired.
          I love thinking about pagelayouts [... and I don't like sudokus].

          Make sure that modifications of blocklib.php and themes are applied simultaneously!

          Good luck for the integration process!

          Cheers
          Dietmar

          Show
          Dietmar Wagner added a comment - Hi Mary, the pleasure is all mine! I'm glad I could help and I will help further on if my help is desired. I love thinking about pagelayouts [... and I don't like sudokus] . Make sure that modifications of blocklib.php and themes are applied simultaneously! Good luck for the integration process! Cheers Dietmar
          Hide
          Mary Evans added a comment -

          Hi Dietmar,

          Some thoughts on a Sunday Morning:

          A thought occured to me last night, after I had switched off my computer. I think we should add the .blocks-moving rule to Canvas pagelayout.css too, as this theme uses a different sideblock setting of 210px compared with 200px set in Base theme.

          Some observations, which Theme designers need to be aware of. If a theme has Base and Canvas as parent themes, and that they are put in the correct order for example:

          $THEME->parents = array('canvas','base') 
          

          the rules (if we apply them to Canvas) will apply to the theme, as Canvas theme will take precedence over Base theme.

          If on the other hand, the parent themes have been put in the wrong order, for example:

          $THEME->parents = array('base','canvas') 
          

          the rules set in Canvas will NOT be applied as Base theme will take precedence.

          Of course if any theme blocks pagelayout.css (eg. Magazine and Afterburner) from either/or both themes, then the rules need to be applied to the theme's css, as we have done in the case of Afterburner and Magazine.

          If a theme has got the order wrong (eg: Formal White) and has EXCLUDED Canvas theme's pagelayout.css, then the rules will come from Base.

          But if a similar situation occurs in another theme, where Base & Canvas are set as parent thenes, but that theme excludes Base pagelayout and has not made provision for block-moving in the theme's css, then that theme will fail unless we add BLOCKS-MOVING CSS rules in Canvas pagelayout.css.

          Does this make sense?

          I'll start getting the GIT branch set up now so I can commit all the files together later today.

          Cheers
          Mary

          Show
          Mary Evans added a comment - Hi Dietmar, Some thoughts on a Sunday Morning: A thought occured to me last night, after I had switched off my computer. I think we should add the .blocks-moving rule to Canvas pagelayout.css too, as this theme uses a different sideblock setting of 210px compared with 200px set in Base theme. Some observations, which Theme designers need to be aware of. If a theme has Base and Canvas as parent themes, and that they are put in the correct order for example: $THEME->parents = array('canvas','base') the rules (if we apply them to Canvas) will apply to the theme, as Canvas theme will take precedence over Base theme. If on the other hand, the parent themes have been put in the wrong order, for example: $THEME->parents = array('base','canvas') the rules set in Canvas will NOT be applied as Base theme will take precedence. Of course if any theme blocks pagelayout.css (eg. Magazine and Afterburner) from either/or both themes, then the rules need to be applied to the theme's css, as we have done in the case of Afterburner and Magazine. If a theme has got the order wrong (eg: Formal White) and has EXCLUDED Canvas theme's pagelayout.css, then the rules will come from Base. But if a similar situation occurs in another theme, where Base & Canvas are set as parent thenes, but that theme excludes Base pagelayout and has not made provision for block-moving in the theme's css, then that theme will fail unless we add BLOCKS-MOVING CSS rules in Canvas pagelayout.css. Does this make sense? I'll start getting the GIT branch set up now so I can commit all the files together later today. Cheers Mary
          Hide
          Dietmar Wagner added a comment -

          Hmmm,

          a very important question, Mary. I'm sorry for being late but I needed some time to check the scenarios.

          @ core themes

          If you have several parent themes for one theme and a set of competetive rules for a website element the one with the highest specifity will get the advantage (regardless of the position of the corresponding theme in the order of parent themes).
          As our .blocks-moving rules have a relativ high specifity (detailed path) and all actual core themes use BASE as a parent theme everything should be ok for the core themes.

          @ non core themes

          I think there ist no theme that excludes BASE or BASE's pagelayout.css on the one hand but takes CANVAS as a parent theme at the other hand. So I think you could add some rules to CANVAS for moving blocks but it's not indispensible.

          Another proposal: In base/style/dock.css you can find the following pointer:

          /**

          • Whilst the dock isn't supported by the base theme this CSS is here so that those
          • themes that do want to use the dock will have a starting point at least

          */

          After reading this a developer knows that he has to care for the dock. Perhaps something like that in base/style/pagelayout.css and at http://docs.moodle.org/dev/Themes_2.0 and/or http://docs.moodle.org/dev/Themes_2.0_creating_your_first_theme could be good idea.

          Hope that helps.

          Cheers
          Dietmar

          Show
          Dietmar Wagner added a comment - Hmmm, a very important question, Mary. I'm sorry for being late but I needed some time to check the scenarios. @ core themes If you have several parent themes for one theme and a set of competetive rules for a website element the one with the highest specifity will get the advantage (regardless of the position of the corresponding theme in the order of parent themes). As our .blocks-moving rules have a relativ high specifity (detailed path) and all actual core themes use BASE as a parent theme everything should be ok for the core themes. @ non core themes I think there ist no theme that excludes BASE or BASE's pagelayout.css on the one hand but takes CANVAS as a parent theme at the other hand. So I think you could add some rules to CANVAS for moving blocks but it's not indispensible. Another proposal: In base/style/dock.css you can find the following pointer: /** Whilst the dock isn't supported by the base theme this CSS is here so that those themes that do want to use the dock will have a starting point at least */ After reading this a developer knows that he has to care for the dock. Perhaps something like that in base/style/pagelayout.css and at http://docs.moodle.org/dev/Themes_2.0 and/or http://docs.moodle.org/dev/Themes_2.0_creating_your_first_theme could be good idea. Hope that helps. Cheers Dietmar
          Hide
          Mary Evans added a comment -

          Methinks... wir erreicht haben den Punkt von nicht Rückkehr!!!

          Show
          Mary Evans added a comment - Methinks... wir erreicht haben den Punkt von nicht Rückkehr!!!
          Hide
          Mary Evans added a comment -

          @Deitmar - I had some problems trying to add this all to Moodle-20_Stable and was not until I realised Afterburner did not appear until Moodle 2.1 so not sure if we need to worry too much about Moodle 2.0 or do we?

          Show
          Mary Evans added a comment - @Deitmar - I had some problems trying to add this all to Moodle-20_Stable and was not until I realised Afterburner did not appear until Moodle 2.1 so not sure if we need to worry too much about Moodle 2.0 or do we?
          Hide
          Mary Evans added a comment -

          All done! I've just added that last commit as a separate item.

          I can sleep now!

          Cheers
          Mary

          Show
          Mary Evans added a comment - All done! I've just added that last commit as a separate item. I can sleep now! Cheers Mary
          Hide
          Dietmar Wagner added a comment -

          1+ for you, Mary!

          Thanks a lot!
          Dietmar

          Show
          Dietmar Wagner added a comment - 1+ for you, Mary! Thanks a lot! Dietmar
          Hide
          Aparup Banerjee added a comment -

          This one's for you SamH

          Show
          Aparup Banerjee added a comment - This one's for you SamH
          Hide
          Aparup Banerjee added a comment -

          Hi Mary,
          I had glanced through the patch and i'll admit it did take me awhile to find the actual fix to the problem in this large patch. The tabs and whitespace fixes were very distracting .

          It would help us immensely if you could separately commit all white space related changes during your development into another commit. That way we can separate 'fixing' commits from 'tidying up' commits. This would help us understand your fix much more better (and quicker).

          cheers,
          Aparup

          Show
          Aparup Banerjee added a comment - Hi Mary, I had glanced through the patch and i'll admit it did take me awhile to find the actual fix to the problem in this large patch. The tabs and whitespace fixes were very distracting . It would help us immensely if you could separately commit all white space related changes during your development into another commit. That way we can separate 'fixing' commits from 'tidying up' commits. This would help us understand your fix much more better (and quicker). cheers, Aparup
          Hide
          Mary Evans added a comment - - edited

          Hi Aparup,

          I saw that too after I did the commit and checked out the comparison with master and wondered where all the changes had come from. When I added the the extra CSS for the patch the page looked fine to me and I made NO changes to the structure of the previous CSS mark-up. That is the honest truth. However, Text-Pad (my text editor) is set up so all tabs are 4 spaces and all white spaces deleted,so my conclusion is that it was Text-Pad that cleaned up the page.

          If you look at the original Magazine layout.css the tabs are wider, I think about 8 spaces.

          Now I know that it is more important to just ADD THE PATCH I'll do the changes in NotePad in future and make sure I don't mess stuff up again!

          Cheeers and thanks.
          Mary

          Show
          Mary Evans added a comment - - edited Hi Aparup, I saw that too after I did the commit and checked out the comparison with master and wondered where all the changes had come from. When I added the the extra CSS for the patch the page looked fine to me and I made NO changes to the structure of the previous CSS mark-up. That is the honest truth. However, Text-Pad (my text editor) is set up so all tabs are 4 spaces and all white spaces deleted,so my conclusion is that it was Text-Pad that cleaned up the page. If you look at the original Magazine layout.css the tabs are wider, I think about 8 spaces. Now I know that it is more important to just ADD THE PATCH I'll do the changes in NotePad in future and make sure I don't mess stuff up again! Cheeers and thanks. Mary
          Hide
          Mary Evans added a comment -

          @Sam
          I know you are very, very, very, busy Sam, and I am loathed to keep pestering you, but having just found out how to do Sub-Tasks, I am just concerned now that I should have perhaps added each of the proposed changes in this tracker issue as Sub-Tasks.

          Would it help if I made these into Sub-Tasks? Or are they OK as they are?

          Thanks
          Mary

          Show
          Mary Evans added a comment - @Sam I know you are very, very, very, busy Sam, and I am loathed to keep pestering you, but having just found out how to do Sub-Tasks, I am just concerned now that I should have perhaps added each of the proposed changes in this tracker issue as Sub-Tasks. Would it help if I made these into Sub-Tasks? Or are they OK as they are? Thanks Mary
          Hide
          Sam Hemelryk added a comment -

          Hi Mary,

          Sub tasks are great if you have a single big task that can be individual small tasks, we use them a lot for the development of new features because we can create a sub task for each thing we need the new feature to address e.g. backup, restore, creating reports...
          Sometimes as was the case the other day for you we use them if we need to make similar changes to several areas of code within Moodle. In your case you had similar changes that needed to be made on each theme, in which case creating a subtask for each theme is great because it allows you to easily track where you are up to.

          As for the theme issues here personally at the moment I would just suffice with linking them together. The exception to this is perhaps that if you find that several of them share an common solution and you would like to try to fix all of them for an upcoming/future release then creating a meta issue and make them subtasks is perhaps a good idea as it will allow you to easily keep track of your progress... really it is up to you

          In regards to these changes by the way thank you very much (and Dietmar as well). Certainly digging down into this issue, finding the root of this issue and then patching it has paid off and you have managed to fix the same issue in many theme's with a small changeset.
          I'll continue my review now but certainly at this point it looks like this will go in no probs!

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi Mary, Sub tasks are great if you have a single big task that can be individual small tasks, we use them a lot for the development of new features because we can create a sub task for each thing we need the new feature to address e.g. backup, restore, creating reports... Sometimes as was the case the other day for you we use them if we need to make similar changes to several areas of code within Moodle. In your case you had similar changes that needed to be made on each theme, in which case creating a subtask for each theme is great because it allows you to easily track where you are up to. As for the theme issues here personally at the moment I would just suffice with linking them together. The exception to this is perhaps that if you find that several of them share an common solution and you would like to try to fix all of them for an upcoming/future release then creating a meta issue and make them subtasks is perhaps a good idea as it will allow you to easily keep track of your progress... really it is up to you In regards to these changes by the way thank you very much (and Dietmar as well). Certainly digging down into this issue, finding the root of this issue and then patching it has paid off and you have managed to fix the same issue in many theme's with a small changeset. I'll continue my review now but certainly at this point it looks like this will go in no probs! Cheers Sam
          Hide
          Sam Hemelryk added a comment -

          Thanks Mary - this has been integrated now.
          Just noting I had to cherry-pick the 21 branch. It looks like you may have based that branch on master rather than 21 by mistake. No probs though

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Thanks Mary - this has been integrated now. Just noting I had to cherry-pick the 21 branch. It looks like you may have based that branch on master rather than 21 by mistake. No probs though Cheers Sam
          Hide
          Mary Evans added a comment -

          Hi Sam!

          I don't know what happened with that branch, one minute it was all working great next minute all sorts of things happened. When I pushed it to to the the GitHub it was as you said the master. So I deleted it and started over, I must have got my branch lines crossed. I thought I had fixed it...obviously not!

          It can be a bit of a GIT sometimes...especially when it's important that you get it right. I was perhaps trying to hard.

          Anyway...thanks for your help in getting this integrated...it is much appreciated by both myself and Dietmar who did all the digging to find the root of this problem.

          Cheers
          Mary

          Show
          Mary Evans added a comment - Hi Sam! I don't know what happened with that branch, one minute it was all working great next minute all sorts of things happened. When I pushed it to to the the GitHub it was as you said the master. So I deleted it and started over, I must have got my branch lines crossed. I thought I had fixed it...obviously not! It can be a bit of a GIT sometimes...especially when it's important that you get it right. I was perhaps trying to hard. Anyway...thanks for your help in getting this integrated...it is much appreciated by both myself and Dietmar who did all the digging to find the root of this problem. Cheers Mary
          Hide
          Rajesh Taneja added a comment -

          Works Great
          No problem in docking/undocking/moving blocks

          Thanks for fixing this Mary.

          Show
          Rajesh Taneja added a comment - Works Great No problem in docking/undocking/moving blocks Thanks for fixing this Mary.
          Hide
          Dietmar Wagner added a comment -

          Phew,
          it would seem that we are through with all .side-post-only issues, Mary.
          My thanks to you, Sam and Rajesh! Working here was a great pleasure for me!
          Dietmar

          Show
          Dietmar Wagner added a comment - Phew, it would seem that we are through with all .side-post-only issues, Mary. My thanks to you, Sam and Rajesh! Working here was a great pleasure for me! Dietmar
          Hide
          Mary Evans added a comment -

          Thank you all...so pleased to see this made it!

          Cheers
          Mary

          Show
          Mary Evans added a comment - Thank you all...so pleased to see this made it! Cheers Mary
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Yes, you got this finally upstream, just in time for Moodle 2.2beta. Congrats and thanks!

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Yes, you got this finally upstream, just in time for Moodle 2.2beta. Congrats and thanks! Ciao
          Hide
          Mary Evans added a comment -

          SuperDuperTrooper!
          Thanks

          Show
          Mary Evans added a comment - SuperDuperTrooper! Thanks

            People

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

              Dates

              • Created:
                Updated:
                Resolved: