Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
Followup from #1039666: Placing a block in a region via select dropdown moves it to the top of the region, but it will show at the bottom
When there are no blocks in the last region (that would be the Disabled region), you can't move a block to that region using the 'Region' select drop-down.
Target region has blocks:
Target region is empty:
Steps to reproduce
- Move all blocks from the last region (Disabled) to other regions.
- Move one block back to the last region using the region select drop-down.
Proposed resolution
Fix the block placement in the last region (eg. Disabled) when there are no blocks in that region.
Remaining tasks
- Write a patch
- Review
User interface changes
When selecting 'Disabled' in a block region, the block is placed in the 'Disabled' region.
API changes
None
Comments
Comment #1
czigor CreditAttribution: czigor commentedFor me the patch from https://drupal.org/comment/6908112#comment-6908112 works. (Credits go to dozymoe.)
Comment #2
droplet CreditAttribution: droplet commentedComment #3
idebr CreditAttribution: idebr commentedThis still occurs in Drupal 8 as well as Drupal 7, so it should be fixed in Drupal 8 first and then backported to 7.
Attached patch ports #1 to Drupal 8.
Comment #4
idebr CreditAttribution: idebr commentedComment #5
tisteegz CreditAttribution: tisteegz commented@idebr I can't seem to replicate this issue in Drupal 8? I had no blocks disabled and then using the region select list I managed to disable it correctly.
Comment #6
idebr CreditAttribution: idebr commented@tisteegz Thanks for having a look! Indeed, disabling a block still works. The problem is limited to the display in the UI: when selecting a different region, a block is moved to the bottom of the target region. However, when the target region is 'Disabled' and there are no other disabled blocks this fails. I have updated the issue summary with two screencaps to clarify the problem.
Comment #7
xjmComment #8
mgiffordNeeds re-roll.
Comment #9
oo0shiny CreditAttribution: oo0shiny as a volunteer commentedRerolled patch with code from idebr.
Comment #10
oo0shiny CreditAttribution: oo0shiny as a volunteer commentedComment #11
Jeff Cardwell CreditAttribution: Jeff Cardwell as a volunteer commentedI was able to reproduce the UI issue. After applying the patch in #9, selecting -none- (when no other blocks are already present in "Disabled") for a block results in the block being moved to the appropriate "Disabled" section in the UI. The patch looks like it works.
Comment #12
bdimaggioThis looks and works great in Chrome 47, FF 42, Safari 9.0.3, IE 9, IE 10. This patch just fixes up formatting.
Comment #13
alexpottI've manually tested the patch and it works. I've also run the code against our eslint rules and it passes.
However, I think this line is now obsolete because of the code added by this patch. The row is placed (in the correct position) by the new code.
Comment #14
drintios CreditAttribution: drintios commentedComment #15
drintios CreditAttribution: drintios at Skilld commentedRemoved line suggested by alexpott.
Comment #16
drintios CreditAttribution: drintios commentedComment #17
andypostplease don't remove useful comment
Comment #18
drintios CreditAttribution: drintios commentedComment #19
drintios CreditAttribution: drintios at Skilld commentedReturned comment
Comment #20
samiullah CreditAttribution: samiullah at Srijan | A Material+ Company commentedComment #21
samiullah CreditAttribution: samiullah at Srijan | A Material+ Company commentedAfter applying above patch if we move a block to empty region using dropdown, it is working fine.
Step 1
Step 2
Comment #22
samiullah CreditAttribution: samiullah at Srijan | A Material+ Company commentedComment #23
alexpottCommitted d841acb and pushed to 8.0.x, 8.1.x, 8.2.x. Thanks! Added myself and @samiullah to the credit for reviewing - me for the code review and @samiullah for adding screenshots.
Unfortunately no one who has worked on this issue will get any credit until the patch is ported and committed to Drupal 7 because of the current backport policy.
Comment #27
therealssj CreditAttribution: therealssj commentedHere is a patch for D7.
Comment #28
therealssj CreditAttribution: therealssj commentedComment #29
Triumphent CreditAttribution: Triumphent commentedUnfortunately, this last patch doesn't work for me (ver 7.x.) I still can't move blocks from the block list (I can however, from inside the block) and I cannot move a block in Dashboard (same thing, I need to configure the block and select the region there. That works.)
Comment #34
poker10 CreditAttribution: poker10 at ActivIT s.r.o. commentedI have tested the D7 patch #27 and it is working correctly (you need to clear the cache after applying the patch). Now it is possible to move block to the "disabled" region when there are no blocks.
Comment #35
Rinku Jacob 13 CreditAttribution: Rinku Jacob 13 at Srijan | A Material+ Company for Drupal India Association commentedI think the issue got fixed now, because i have tested the issue in D7 and applied above patch. The patch was successfully applied and issue got solved. Adding Screen record for the reference . Need RTBC +1
Comment #36
poker10 CreditAttribution: poker10 at ActivIT s.r.o. commentedMoving carefully to RTBC (because we do not have JS tests in D7) and adding tag for the second review.
Comment #37
shashank5563 CreditAttribution: shashank5563 at Melity commentedI have tested the D7 patch #27 and it is working fine. So I am moving it RTBC+1.
Comment #39
mcdruidTested this manually and confirmed the fix (once caches are cleared).
I don't think it's worth adding an empty update hook to force a cache clear for this change.
Thank you everyone that contributed!