Bootstrap & Simple theme do not support block drag/drop

This is possibly related to the use of HTML5 tags <aside> for block areas instead of using <div>

Apparently this is probably related to MDL-32608

I think that this is a manifestation of MDL-32608.

Ruslan, is there a workaround for the Bootstrap theme do you think?

Just to confirm that it's the use of "aside" tag instead of "div" that broke this, because that's what the javascript in lib/yui/blocks/blocks.js is looking for.

But there's some complex interaction with other issues here. I'd been having issues with drag'n'drop on mobile devices (though there was some debate in the bug about whether I was imagining it or not, see MDL-38371 and if you give it a try yourself please report back on what you experience) so I'd actually been looking for ways to disable the drag'n'drop javascript and make the non-javascript controls come back, as I though they were a) more suitable and b) less broken on touch devices. The only one I succeeded on bringing back was the block movements, which was an accidental success which occurred as a result of this bug. So "fixing" this by making the javascript not relay on div might actually be a step back for use on touch devices, which might be an initial focus of some users of this theme. So it's tricky.

There is no reason why you can't use the the following code inside the aside container...

<aside class="span3">
    <div id="region-pre" class="block-region">
    <div class="region-content">
    <?php
if (!right_to_left()) {
    echo $OUTPUT->blocks_for_region('side-pre');
} else if ($hassidepost) {
    echo $OUTPUT->blocks_for_region('side-post');
} ?>
    </div>
    </div>
</aside>

blocks.js
line 178
regionname = this.get_region_id(drop.ancestor('div.'+CSS.BLOCKREGION));

just take out the 'div.' and it works - except, as david states on touch screens....

should this issue be in javascript rather than themes?

Hi Mary,

This change works for the bootstrap theme - but also needs to be implemented for the simple theme (which is the one which will be selectable). We really need to keep these two in sync now. I'll add a commit for this now to save a cycle.

Re: fixing the JS selector - that sounds like a good improvement, but does not seem like a blocker for this issue thanks to Marys workaround.

Integrated to master (with support for simple theme).

Thanks, Damyon

Re: extra commit for simple theme - sorry I didn't see MDL-39250 in the queue - I've closed that one as a duplicate now because I fixed it here.

Tested successfully using Simple. Bootstrap is no longer selectable in the theme selector. After asking in dev chat that is apparently ok.

@Damyon: You need to go to Specsavers then!

   _,aad888P""""""""""Y888888888888888888888888888888P"""""""""""Y888baa,_
  aP'                     `""Ybaa,         ,aadP""'                     `Ya
 dP                             `"b,     ,d"'                             Yb
 8l                               8l_____8l                                8l
[8l                               8l"""""8l                                8l]
 8l                              d8       8b                               8l
 8l                             dP/       \Yb                              8l
 8l                           ,dP/         \Yb,                            8l
 8l                        ,adP'/           \`Yba,                         8l
 Yb                     ,adP'                   `Yba,                     dP
  Yb                ,aadP'                         `Ybaa,                dP
   `Yb          ,aadP'                                 `Ybaa,          dP'
     `Ybaaaaad8P"'                                         `"Y8baaaaadP'

as developers i've always thought we should be optimising, not increasing code.

surely instead of adding more dom elements to every page, removing one bug in the javascript is a huge win?

i'm sure i read that moodle guide for integration says code has to be better, not worse.

@mary ?

your code is a workaround, not a fix.

the fix is to fix the js so it works and no theme has to do this workaround.

grep and replaced 'div.'+CSS.BLOCKREGION with CSS.BLOCKREGION - should do the trick?

Nice art Mary!

Stuart - I think the div/aside issue will be dealt with in MDL-32608 - but that is a riskier change and we are in code freeze now so I felt it was better to integrate this solution. From reading MDL-32608 - there may be more changes required than just changing that selector (I think the js dynamically generates those block containers in some cases).

That may be so Stuart, but as I see it we are regressing with some of the Bootstrap changes, besides Rome was not built in a day, so please bear with us until other things are fixed.

Thanks.

@stuart,

I have tried the fix for JavaScript but it did not seem to work. Did your fix work for Moodle 2.5?

the issue i'm seeing, even with mary's https://github.com/lazydaisy/moodle/compare/master...MDL-38898_master is that the js adds a dom element when there is no post-region.

where it appends this in the dom breaks the bootstrap layout.

to reproduce :
turn editing on in a site with only pre-region
try and drag a block into the js appended post-region

results :
in the dom the js appends the post-region after the pre-region which breaks the layout.

this would be the case for this js with any theme/layout with multiple regions that didn't use something like the http://alistapart.com/article/holygrail layout.

Can you create a new bug issue for this. As I think it is the same as one that is jinxed the Holy Grail too.

I feel myself really alone tonight! So was time to push your fixes upstream!

"Lest we forget. We will remember them."

Thanks and ciao!

@Eloy: There is an error in Clean/layout/general.php probably caused by Damyon when he added fix to Simple/layout/general.php when Integrating this issue. For some reason he missed my patch in MDL-39250 which he clearly did not check.

Should I fix this in MDL-39250 if I change subject title to read Clean theme does not support drag-n-drop?

I need to reopen MDL-39250 as Damyon closed it as a duplicate!

I knew this would happen!

I think Stuart that the problem is in the CSS.
The fact body classes are generated seems to have been overlooked in bootstrap as a means to identify different layouts. eg., .side-pre-only, .side-post-only, and .content-only. .blocks-moving.side-pre-only, .blocks-moving.side-post-only

i'm still confused why this issue is closed and fixed.

mary?

The fact it is closed means it has been integrated, faults and all and is beyond return.
I fixed the problem I found in MDL-39250.

However, you are correct with the bug you have found in that the drag'n'drop tries to find a block region is a page where none exist, because the region is not listed in the config.php, layout/frontpage being a prime example.

This is now being tracked in MDL-39382

by adding the extra divs to the dom it breaks the ability to move blocks from one region to another when only one region has blocks in.

this is true even when the config is set for two regions.

its odd this didn't show up in testing -leaving this bug in seems much more serious than not having drag n drop.

Hi Stuart,

I noticed this odd behaviour as well, but since this issue is closed can you open another tracker so we can look at this new problem in detail, and try and find a fix for it too?

I think, like you have said in the past, it's the way the JS is written for the holy-grail layout, and so it fails if the layout is different.

Thanks
Mary

Hi folks, sorry am so late to this conversation. I installed the latest 2.5 this afternoon. Not being able to move a block into an empty column is a massive issue from an admin user's or course creator's perspective. It totally breaks Moodle and should surely be considered a blocker, yet this issue appears to be 'fixed' and 'closed'?! I am working with a new test site with just a couple of sample courses, and it restricts me to a 2 column layout, I cannot add a right hand column at all. Hugely restrictive to end users.

Does it really need a new ticket opening? As I see it the ticket was opened, tested, fixed and closed however testing missed the bug, hence the ticket should just be reopened. We already have a closed duplicate at MDL-39250 and what looks like another duplicate at #MDL-39382, do we really need a 4th ticket for the same issue? I am no developer though, maybe there are subtle differences in these three tickets, but they look the same to my non-developer eye

Anyway, if a new ticket is what it will take to fix it, I will gladly open one, if that is all that is preventing this being fixed. Please let me know if you want me to do that.

Thanks

As it would be best to get this resolved asap I've raised MDL-39573.