Moodle

Remove wrongly placed region-pre ids in aside tags

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Blocker Blocker
  • Resolution: Fixed
  • Affects Version/s: 2.5
  • Fix Version/s: 2.5
  • Component/s: Themes
  • Labels:

Description

Like Base theme, both Bootstrap the Simple themes MUST be able to use drag and drop.

Issue Links

blocks

Sub-task - Issues are sometimes broken into multiple sub-tasks. MDL-39341 Rename 'bootstrap' theme to 'bootstrapbase'

  • Blocker - Blocks development and/or testing, prevents Moodle from running (applicable to bugs only)
  • Closed - The issue is considered finished, the resolution is correct. Issues which are closed should not be reopened.

Sub-task - Issues are sometimes broken into multiple sub-tasks. MDL-39231 Bootstrap layout file general.php region-bs-main

  • Minor - Minor loss of function where workaround is possible
  • Closed - The issue is considered finished, the resolution is correct. Issues which are closed should not be reopened.
has a non-specific relationship to

Sub-task - Issues are sometimes broken into multiple sub-tasks. MDL-38898 Bootstrap & Simple theme do not support block drag/drop

  • Blocker - Blocks development and/or testing, prevents Moodle from running (applicable to bugs only)
  • Closed - The issue is considered finished, the resolution is correct. Issues which are closed should not be reopened.

Bug - A problem which impairs or prevents Moodle from functioning correctly. MDL-32608 Block regions need to be wrapped in a generic div that is not theme dependent

  • Minor - Minor loss of function where workaround is possible
  • Open - The issue is open and ready for the assignee to start work on it.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Damyon Wiese added a comment -

Hmm - oops. I didn't see this in the queue when integrating MDL-38898 and I fixed it there.

I'll close this as a duplicate (my fault sorry).

Show
Damyon Wiese added a comment - Hmm - oops. I didn't see this in the queue when integrating MDL-38898 and I fixed it there. I'll close this as a duplicate (my fault sorry).
Hide
Permalink
Mary Evans added a comment -

Damyon, looks like you left 2 region-pre ids in the aside tags in the fix you added when you closed this issue without using the original patch when Integration MDL-38898.

I've just pushed the fix to clean up the general.php.

Cheers

Show
Mary Evans added a comment - Damyon, looks like you left 2 region-pre ids in the aside tags in the fix you added when you closed this issue without using the original patch when Integration MDL-38898 . I've just pushed the fix to clean up the general.php. Cheers
Hide
Permalink
Mary Evans added a comment -

Reopening this to fix regression

Show
Mary Evans added a comment - Reopening this to fix regression
Hide
Permalink
Dan Poltawski added a comment -

Hi Mary,

I've looked at this but can't find a way to reproduce the problem? From my reading of the code it seems this only happens when there are no blocks at all? Am I misreading? How can I reproduce this problem?

Show
Dan Poltawski added a comment - Hi Mary, I've looked at this but can't find a way to reproduce the problem? From my reading of the code it seems this only happens when there are no blocks at all? Am I misreading? How can I reproduce this problem?
Hide
Permalink
Mary Evans added a comment -

There isn't a problem as such it is just that Damyon did not remove the id="region-pre" from the aside tag. Look at Bootstrap theme layout/general.php to compare.

It makes me mad to think this would have been a smooth transition had Damyon not messed up the integration. Read MDL-38898 discussion for the story.

Show
Mary Evans added a comment - There isn't a problem as such it is just that Damyon did not remove the id="region-pre" from the aside tag. Look at Bootstrap theme layout/general.php to compare. It makes me mad to think this would have been a smooth transition had Damyon not messed up the integration. Read MDL-38898 discussion for the story.
Hide
Permalink
Mary Evans added a comment -

You can see the commit for bootstrap here...
https://github.com/lazydaisy/moodle/commit/c9ff4d0e0fa679ed7cfddf14456fc91c64131301

Show
Mary Evans added a comment - You can see the commit for bootstrap here... https://github.com/lazydaisy/moodle/commit/c9ff4d0e0fa679ed7cfddf14456fc91c64131301
Hide
Permalink
Damyon Wiese added a comment -

Thanks Mary, sorry for the original mess up.

This has been integrated to master now. I tested it in integration, so I'll pass the test for this.

Show
Damyon Wiese added a comment - Thanks Mary, sorry for the original mess up. This has been integrated to master now. I tested it in integration, so I'll pass the test for this.
Hide
Permalink
Damyon Wiese added a comment -

Works as described.

Thanks Mary!

Show
Damyon Wiese added a comment - Works as described. Thanks Mary!
Hide
Permalink
Mary Evans added a comment -

Great stuff! Thanks you're a star!

Show
Mary Evans added a comment - Great stuff! Thanks you're a star!
Hide
Permalink
Dan Poltawski added a comment -

Thanks! You're changes are now spread to the world through this git and our source control repositories.

No time to rest though, we've got days to make 2.5 the best yet!

ciao

Show
Dan Poltawski added a comment - Thanks! You're changes are now spread to the world through this git and our source control repositories. No time to rest though, we've got days to make 2.5 the best yet! ciao

People

Vote (0)
Watch (4)

Dates

  • Created:
    Updated:
    Resolved: