search

catalyst / moodle-block_multiblock

Moodle plugin which allows you to embed multiple blocks within a single block region using different formats including tabs and accordions
GNU General Public License v3.0
12 stars 12 forks source link

Subblock #85

Closed james-pearce-bath-ac closed 1 year ago

james-pearce-bath-ac commented 1 year ago

Fixed a bug in the get_content function in block_multiblock.php where if the first option in the array of presentation styles is selected this would not be applied as the template as it is 0 and the if statement was matching it as false.

Other changes in this are merging of upstream commits

danmarsden commented 1 year ago

Hi @james-pearce-bath-ac thanks for the PR - I can't see any documentation on what the "subblock" branch does or why it's not a feature merged into the main branch? - are you able to help there?

Also - I'm a little concerned that the version bump on the subblock branch in this PR is quite a bit higher than the version on the main branch - we need to be a bit careful there.

james-pearce-bath-ac commented 1 year ago

Hi @danmarsden, Sorry I may have requested the pull to go into the incorrect branch. This is really a bug fix to a PR I did back in June (https://github.com/catalyst/moodle-block_multiblock/pull/80). The bug was due to an if statement checking a value when it should have been checking if set. I bumped the version number to the date when I did the final commit. I'm happy to amend if needed.

danmarsden commented 1 year ago

ah - no worries - so... looking at this a bit more I think the subblock branch is an old development branch that was squashed and merged into the main branch.... and you just have one commit you're trying to add here (c67923465141de3a532f4cbc75b46077f841e758) - @marxjohnson any objections to deleting the old "subblock" branch in here to avoid future confusion now that it's been merged in?

@james-pearce-bath-ac do you want to open a new PR with that commit just cherry-picked on top of the main branch instead?