Great module. When showing all blocks, there's no indication to which group a block belongs. The attached patch fixes this by adding a new column to the blocks table (only when viewing all blocks) that shows the block visibility group.

I also changed the UI term for blocks belonging to no visibility group from "Unset only" to "Global", because that's easier to understand I think, especially in the table column "Global" is more telling than "Unset" IMO.

CommentFileSizeAuthor
#4 show_visibility_group-2567099-4.patch8.95 KBtedbow
#2 show_visibility_group-2567099-2.patch5.93 KBtedbow
show_block_group.patch3.45 KBFrando

Comments

Frando created an issue. See original summary.

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
StatusFileSize
new show_visibility_group-2567099-2.patch5.93 KB

@Frando thanks for the patch!

I like the change and the wording is better too.

I have changed it slightly to refactor out the new logic into addGroupColumn function

Let me know what you think.

Frando’s picture

Frando commented

Yes, I agree that a seperate method is nicer.

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
StatusFileSize
new show_visibility_group-2567099-4.patch8.95 KB

@Frando, I had another idea I would like to get your(or anyone's) opinion on.

I have added a patch that adds a "Show global blocks when viewing a visibility group." checkbox to the form.

If it is checked then when viewing a group you would see that group's block and the global blocks.

My thought is that when you are viewing a particular group it would make sense to also see what "global" blocks would also show up when viewing a page that matches the condition.

For 2 reasons

  1. To avoid adding duplicate blocks. If the "search form" is already placed globally you probably don't want to place it again(at least not in the same region)
  2. It would make it easier to arrange/sort the blocks in a region. Since they will all be shown together to the end user.

I am using the Drupal 8 state system for this setting rather than config because I think this would be something the site builder might want to turn on and off and not have it affect config export.

The patch will be probably need some clean up but it is just an idea now.

Frando’s picture

Frando commented

Yes, I like the change. I was trying to AJAXify the whole thing (for both changes in the select and the checkbox to just rebuild and reload the block list table), unfortunately it doesn't work, though, due to a strange bug with the core block list form. See #2567595: Block list form cannot be rebuilt via Ajax.

  • tedbow committed 2e58f14 on 8.x-1.x authored by Frando 
    Issue #2567099 by tedbow, Frando: Show visibility group in block layout...
tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Status: Needs review » Fixed

Alright I committed this. Thanks for the help!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.