backdrop / backdrop-issues

Issue tracker for Backdrop core.
144 stars 40 forks source link

function Layout::addBlock position not working properly #6732

Closed stpaultim closed 3 weeks ago

stpaultim commented 1 month ago

Description of the bug

The code to add a block to a layout in a specific position, does not seem to be working properly.

Steps To Reproduce

To reproduce the behavior:

  1. Spin up a new/demo site.

  2. Add and enble the devel module

  3. Use devel module to execute the following php

    $home_layout = layout_load('home');
    $home_layout->addBlock('views', 'promoted-block', 'top', 0);
    $home_layout->save();
  4. Check front page and see that the promoted content block has been added to the END of the top region, instead of as the first item in the top region.

It is possible that it is my syntax that is incorrect. See:

https://docs.backdropcms.org/api/backdrop/core%21modules%21layout%21includes%21layout.class.inc/function/Layout%3A%3AaddBlock/1

layout.class.inc Layout::addBlock($block_module, $block_delta, $region_name, $position = NULL) Add a block to a particular region.

Parameters:

  • string $block_module: The module providing the block to be positioned.
  • string $block_delta: The block delta from hook_block_info().
  • string $region_name: The name of the region within the layout.
  • int $position: The position of the block within the region. If not specified, the block will be the last block within the specified region.

Expected behavior

I expected the Promoted Content block to be the first item in the top region, NOT the last item.

Additional information

It is possible that I've just got the wrong syntax.

argiepiano commented 1 month ago

Yup, that's a bug. The problem is that the if statement in line 537 of layout.class.inc fails when $position is 0.

The correct line should be

if (is_numeric($position)) {

stpaultim commented 1 month ago

I'm giving this the "Good First Issue" tag, because we seem to have a short one line change and just need someone to create a PR. If additional work is required, that might not be good for a first time contributor.

robertgarrigos commented 1 month ago

I'm not strictly a first contributor, but I feel like... you see, I just created a pr to my own repo...

quicksketch commented 3 weeks ago

Excellent, thank you @robertgarrigos! I merged https://github.com/backdrop/backdrop/pull/4888 into 1.x and 1.29.x. Thank you @argiepiano and @stpaultim!

stpaultim commented 3 weeks ago

@robertgarrigos and @argiepiano - Thanks very much for working on this. This will be VERY helpful for recipes!