Moodle
  1. Moodle
  2. MDL-43995

Backport MDL-38923: Cannot dock blocks with Bootstrapbase/clean theme

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.6.1
    • Fix Version/s: 2.6.4
    • Component/s: Themes
    • Labels:

      Description

      backport MDL-38923 to 2.6

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            Andrew Nicols added a comment -

            When this is integrated, the patch from MDL-43994 will also need to be included.

            Show
            Andrew Nicols added a comment - When this is integrated, the patch from MDL-43994 will also need to be included.
            Hide
            Sam Hemelryk added a comment -

            Details added so that this can be backported after a few weeks.

            Show
            Sam Hemelryk added a comment - Details added so that this can be backported after a few weeks.
            Hide
            Marina Glancy added a comment -

            The integration team is considering this backport request right now. Stay tuned!

            Show
            Marina Glancy added a comment - The integration team is considering this backport request right now. Stay tuned!
            Hide
            Sohail Aslam added a comment -

            Hi,
            Not sure this is the right place to ask this question.
            Is this being back ported to 2.5 as well?

            Show
            Sohail Aslam added a comment - Hi, Not sure this is the right place to ask this question. Is this being back ported to 2.5 as well?
            Hide
            Mary Evans added a comment - - edited

            With a bit of luck it could well be a possibility, however HQ are still in discussion about this. So you will need to be patient.

            Show
            Mary Evans added a comment - - edited With a bit of luck it could well be a possibility, however HQ are still in discussion about this. So you will need to be patient.
            Hide
            Sohail Aslam added a comment -

            Thanks Mary for your reply.
            Do you think we need to create a tracker issue for this so that people can vote for it?

            Show
            Sohail Aslam added a comment - Thanks Mary for your reply. Do you think we need to create a tracker issue for this so that people can vote for it?
            Hide
            Mary Evans added a comment -

            This IS the tracker for it...why create another?

            Show
            Mary Evans added a comment - This IS the tracker for it...why create another?
            Hide
            Sohail Aslam added a comment -

            I think I was not clear in my statement. This tracker is to backport the fix to moodle 2.6. Do we need to create a new tracker to backport the fix to moodle 2.5 as well or that will also be managed under the same tracker number.

            Show
            Sohail Aslam added a comment - I think I was not clear in my statement. This tracker is to backport the fix to moodle 2.6. Do we need to create a new tracker to backport the fix to moodle 2.5 as well or that will also be managed under the same tracker number.
            Hide
            Mary Evans added a comment -

            Oh sorry, you could try but I don't think it will be successful.

            Show
            Mary Evans added a comment - Oh sorry, you could try but I don't think it will be successful.
            Hide
            Gareth J Barnard added a comment -

            Hi,

            Before implementing, please see comments of 23rd Feb 2014 on MDL-44074.

            Gareth

            Show
            Gareth J Barnard added a comment - Hi, Before implementing, please see comments of 23rd Feb 2014 on MDL-44074 . Gareth
            Hide
            Gareth J Barnard added a comment -

            MDL-44074 blocks this because of needing to add '.jsenabled.docked-region-side-post.used-region-side-pre' in moodle/responsive.less to ALL media sizes of '.empty-region-side-post.used-region-side-pre'.

            Show
            Gareth J Barnard added a comment - MDL-44074 blocks this because of needing to add '.jsenabled.docked-region-side-post.used-region-side-pre' in moodle/responsive.less to ALL media sizes of '.empty-region-side-post.used-region-side-pre'.
            Hide
            Michael Woods added a comment -

            Any ETA on this one? Would be a very popular fix, especially on our sites.

            Show
            Michael Woods added a comment - Any ETA on this one? Would be a very popular fix, especially on our sites.
            Hide
            Paul Hague added a comment -

            This is true for us. 2.7 will need an OS upgrade so will probably not happen until the summer holidays.

            Show
            Paul Hague added a comment - This is true for us. 2.7 will need an OS upgrade so will probably not happen until the summer holidays.
            Hide
            Sam Hemelryk added a comment -

            When backporting we will need to backport MDL-44944 as well.

            Show
            Sam Hemelryk added a comment - When backporting we will need to backport MDL-44944 as well.
            Hide
            Brian Barnes added a comment -

            When I add the line "$THEME->enable_dock = true;" to the bootstrap base, I get the following javascript error:
            "Uncaught ReferenceError: region is not defined" on "yui_combo.php?m/-1/core/dock/dock-debug.js:764". When I try the same thing with the anomaly theme, the file doesn't appear to be included, yet the dock works.

            When I then change /lib/yui/build/moodle-core-dock/moodle-core-dock-debug.js:763 from "Y.all(SELECTOR.blockregion).each(function(){" to "Y.all(SELECTOR.blockregion).each(function(region){", the dock appears (when I add something to it) and largely works (although not to the standard of how it works in Anomaly). I haven't been able to look further into this as I haven't yet had a lot of experience with YUI

            Is this what others are experiencing and is it related to this issue?

            Show
            Brian Barnes added a comment - When I add the line "$THEME->enable_dock = true;" to the bootstrap base, I get the following javascript error: "Uncaught ReferenceError: region is not defined" on "yui_combo.php?m/-1/core/dock/dock-debug.js:764". When I try the same thing with the anomaly theme, the file doesn't appear to be included, yet the dock works. When I then change /lib/yui/build/moodle-core-dock/moodle-core-dock-debug.js:763 from "Y.all(SELECTOR.blockregion).each(function(){" to "Y.all(SELECTOR.blockregion).each(function(region){", the dock appears (when I add something to it) and largely works (although not to the standard of how it works in Anomaly). I haven't been able to look further into this as I haven't yet had a lot of experience with YUI Is this what others are experiencing and is it related to this issue?
            Hide
            Eloy Lafuente (stronk7) added a comment - - edited

            This backport request has been voted on by integrators, and we'd like to see it backported asap.

            Thanks!

            Show
            Eloy Lafuente (stronk7) added a comment - - edited This backport request has been voted on by integrators, and we'd like to see it backported asap. Thanks!
            Hide
            Michael Woods added a comment -

            Great, Eloy!

            Show
            Michael Woods added a comment - Great, Eloy!
            Hide
            Brian Barnes added a comment -

            A back ported version can be found at https://github.com/totara/moodle/tree/m26_MDL-43995 with the diff being at https://github.com/totara/moodle/commit/3cf982e375a48db965181902d79e9b4686cb04ec.

            Note: That patch doesn't contain a re-minified version of javascript so isn't quite production ready.

            Show
            Brian Barnes added a comment - A back ported version can be found at https://github.com/totara/moodle/tree/m26_MDL-43995 with the diff being at https://github.com/totara/moodle/commit/3cf982e375a48db965181902d79e9b4686cb04ec . Note: That patch doesn't contain a re-minified version of javascript so isn't quite production ready.
            Hide
            David Mudrak added a comment -

            For the record, Aparup Banerjee managed to backport the solution for our new moodle.org theme (which is bootstrap based). He might provide some feedback, too.

            Show
            David Mudrak added a comment - For the record, Aparup Banerjee managed to backport the solution for our new moodle.org theme (which is bootstrap based). He might provide some feedback, too.
            Hide
            Aparup Banerjee added a comment -

            yes, the fix (as was on moodle.org-2.6) is
            https://github.com/nebgor/moodle/compare/0e088aec08d20bb4ffc596d11dc7d67bb9fdb135...moodle.org-2.6-theme_bootstrapbase-hack-docking_backport_MDL-38923
            we're upgrading moodle.org in MDLSITE-3002, thanks to the proper fix in core - bye-bye hack

            Show
            Aparup Banerjee added a comment - yes, the fix (as was on moodle.org-2.6) is https://github.com/nebgor/moodle/compare/0e088aec08d20bb4ffc596d11dc7d67bb9fdb135...moodle.org-2.6-theme_bootstrapbase-hack-docking_backport_MDL-38923 we're upgrading moodle.org in MDLSITE-3002 , thanks to the proper fix in core - bye-bye hack
            Hide
            Sam Hemelryk added a comment -

            Thanks for sharing all the patches guys - and glad to know this is being used in production, it makes me feel better about it.

            Submitting the direct backport for integration now.

            Show
            Sam Hemelryk added a comment - Thanks for sharing all the patches guys - and glad to know this is being used in production, it makes me feel better about it. Submitting the direct backport for integration now.
            Hide
            Damyon Wiese added a comment -

            Sam - I'm checking that this patch contains all of the followup fixes from regressions.

            Can you please check the following differences I found to what is the current version of this CSS on master ?

            theme/bootstrapbase/less/moodle/responsive.less: (around 170)
            missing this selector:
            .jsenabled.docked-region-side-post.used-region-side-pre

            Same file - same selector missing around line 210

            I'm guessing these are related to: MDL-44074
            Same file - same selector missing around line 510

            Show
            Damyon Wiese added a comment - Sam - I'm checking that this patch contains all of the followup fixes from regressions. Can you please check the following differences I found to what is the current version of this CSS on master ? theme/bootstrapbase/less/moodle/responsive.less: (around 170) missing this selector: .jsenabled.docked-region-side-post.used-region-side-pre Same file - same selector missing around line 210 I'm guessing these are related to: MDL-44074 Same file - same selector missing around line 510
            Hide
            Damyon Wiese added a comment -

            Reopening this for this week Sam (please comment/fix and resubmit)

            Thanks!

            Show
            Damyon Wiese added a comment - Reopening this for this week Sam (please comment/fix and resubmit) Thanks!
            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
            Sam Hemelryk added a comment -

            Ah thanks for picking that up Damyon - I missed the differences in responsive.less between the 26 and master branch when producing this branch for backport.
            I've gone through and applied the diff now and those 3 lines have now being added.

            Cheers
            Sam

            Show
            Sam Hemelryk added a comment - Ah thanks for picking that up Damyon - I missed the differences in responsive.less between the 26 and master branch when producing this branch for backport. I've gone through and applied the diff now and those 3 lines have now being added. Cheers Sam
            Hide
            Damyon Wiese added a comment -

            Sam - there must be a less error somewhere because the moodle.css file is 0 bytes in your branch.

            Can you take a look?

            Thanks!

            Show
            Damyon Wiese added a comment - Sam - there must be a less error somewhere because the moodle.css file is 0 bytes in your branch. Can you take a look? Thanks!
            Hide
            Sam Hemelryk added a comment -

            Ah sorry dude - pushed up the fixed branch now.

            Show
            Sam Hemelryk added a comment - Ah sorry dude - pushed up the fixed branch now.
            Hide
            Damyon Wiese added a comment -

            Thanks Sam,

            Integrated to 26 only.

            Off to testing...

            Show
            Damyon Wiese added a comment - Thanks Sam, Integrated to 26 only. Off to testing...
            Hide
            Frédéric Massart added a comment -

            Passing. However I raised MDL-45646 and MDL-45647 which also occur on master.

            Show
            Frédéric Massart added a comment - Passing. However I raised MDL-45646 and MDL-45647 which also occur on master.
            Hide
            Eloy Lafuente (stronk7) added a comment -

            So, finally, this landed, making Moodle better, many thanks!

            Be patient and understanding.
            Life is too short to be
            vengeful or malicious.

            Phillips Brooks

            Closing as fixed, ciao

            Show
            Eloy Lafuente (stronk7) added a comment - So, finally, this landed, making Moodle better, many thanks! Be patient and understanding. Life is too short to be vengeful or malicious. Phillips Brooks Closing as fixed, ciao

              People

              • Votes:
                34 Vote for this issue
                Watchers:
                37 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: