Problem/Motivation

Blocks have a region and a weight. On the block admin, you can move a block to a different region using the pulldown in the region column. If you do this, the block gets moved to the last row of the new region. BUT, the weight is unchanged. After save, the block displays in the new region based on the old weight.

Proposed resolution

Patch attached

Remaining tasks

Merge the fix

User interface changes

No changes in user interface.

API changes

No changes in API

Data model changes

No changes in Data Model

Original report by JasonSafro

Blocks have a region and a weight. On the block admin, you can move a block to a different region using the pulldown in the region column. If you do this, the block gets moved to the last row of the new region. BUT, the weight is unchanged. After save, the block displays in the new region based on the old weight.

CommentFileSizeAuthor
#7 patch-v3.2790245.diff1.58 KBJasonSafro
#5 patch-v2.2790245.diff1.95 KBJasonSafro
#3 patch.2790245.diff1.28 KBJasonSafro
Untitled.png198.2 KBJasonSafro
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

JasonSafro created an issue. See original summary.

JasonSafro’s picture

JasonSafro CreditAttribution: JasonSafro as a volunteer commented

I think this resolved the issue. This code goes into modules/block/block.js around line 118:

        tableDrag.rowObject = new tableDrag.row(row);

        //-----[ BEGIN BUG FIX ]-----
        // Get the weight of the last pre-existing row
        var last_pre_existing_row_weight = table.find('.region-' +select[0].value + '-message').nextUntil( '.region-message' ).last().prev().find('select.block-weight option:selected').attr('value');
        
        // Set the weight of this row to be greater
        row.find('select.block-weight option:selected').removeAttr('selected');
        row.find('select.block-weight option[value="'+last_pre_existing_row_weight+'"]').next().prop('selected',true);
        //-----[ END BUG FIX ]-----
        
        // // Find the correct region and insert the row as the last in the region.
        table.find('.region-' + select[0].value + '-message').nextUntil('.region-message').last().before(row);
JasonSafro’s picture

JasonSafro CreditAttribution: JasonSafro as a volunteer commented
FileSize
patch.2790245.diff1.28 KB
JasonSafro’s picture

JasonSafro CreditAttribution: JasonSafro as a volunteer commented
Assigned: Unassigned » JasonSafro
Status: Active » Needs review
JasonSafro’s picture

JasonSafro CreditAttribution: JasonSafro as a volunteer commented
FileSize
patch-v2.2790245.diff1.95 KB

Cleaned up the jQuery

Status: Needs review » Needs work

The last submitted patch, 5: patch-v2.2790245.diff, failed testing.

JasonSafro’s picture

JasonSafro CreditAttribution: JasonSafro as a volunteer commented
FileSize
patch-v3.2790245.diff1.58 KB

Fixed whitespace issue w patch.

JasonSafro’s picture

JasonSafro CreditAttribution: JasonSafro as a volunteer commented
Status: Needs work » Needs review
zombree’s picture

zombree CreditAttribution: zombree as a volunteer commented

I'm seeing this issue as well, and can confirm that the patch in #7 fixed it for me.

ranjeesh’s picture

ranjeesh CreditAttribution: ranjeesh as a volunteer commented

I see this bug too.

I confirm that the patch-v3.2790245 in (#7) fixed the issue on my Drupal 7 Site running on PHP 5.6.19 and Mysql 5.5.48.

ranjeesh’s picture

ranjeesh CreditAttribution: ranjeesh as a volunteer commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community
Issue tags: +block, +block weight, +weight
SwapS’s picture

SwapS CreditAttribution: SwapS as a volunteer and at Tech Mahindra commented

Hi JasonSafro,

For me , Functionality looks be working as expected.
There are two cases :
1) If weights are hidden (Which is the default behaviour)on the block configuration page. User can just pull the Block down/up to any of the region ( at any position as required ) . In this case, Block weight also gets updated based on the position of block.
2 ) If weights are visible ( by clicking on 'Show row weights' link on the right - Image attached) , User is expected to select both Region and Weight value to change the block position .

Functionality is working as expected.
Moreover, As per your patch - Block will always be put at the last position in the new region which in turn can break the functionality.(breaking its configuration feature)

       // Set the weight of this row to be greater than the last element in the destination region
       row.find('select.block-weight option:selected').removeAttr('selected');
      row.find('select.block-weight option[value="'+last_row_weight_in_destination_region+'"]').next().attr('selected',true);

       // Find the correct region and insert the row as the last in the region.
    table.find('.region-' + select[0].value + '-message').nextUntil('.region-message').last().before(row);
     first_row_after_destination_region.before(row);

I hope that Clarifies .

SwapS

SwapS’s picture

SwapS CreditAttribution: SwapS as a volunteer and at Tech Mahindra commented
Status: Reviewed & tested by the community » Closed (cannot reproduce)
SwapS’s picture

SwapS CreditAttribution: SwapS as a volunteer and at Tech Mahindra commented
Status: Closed (cannot reproduce) » Closed (works as designed)
David_Rothstein’s picture

David_Rothstein CreditAttribution: David_Rothstein as a volunteer commented
Status: Closed (works as designed) » Needs review

@SwapS, the screenshots definitely show a bug here (which I tested and was able to reproduce just now). The block should not be in a different position after you save than it looks like it's going to be in before you save.

Anyone know if this bug affects Drupal 8 also?

SwapS’s picture

SwapS CreditAttribution: SwapS as a volunteer and at Tech Mahindra commented

Hi David,

Why will weight change when your are not changing/updating it explicitly.
In the screenshots as well, if you see weight are enabled (because of which we are able to see it as -32 in the screenshot.)

Now, When you save the configuration page - New values i.e. Region and weight (which in case of screenshot is -32 again) get saved.

This is perfect behavior i feel.

On the other side, if you drag blog to different place in that case javascript logic in block module takes care of updating weight as well.

SwapS