Custom attributes in panels blocks and variants

I tested this and it looks like it's working. Many thanks!

Lets work on adding some tests for the functionality.

+++ b/src/Form/PanelsBlockConfigureFormBase.php
@@ -135,6 +135,19 @@ abstract class PanelsBlockConfigureFormBase extends FormBase {
+      '#default_value' => !empty($settings['css_classes']) ? $settings['css_classes'] : NULL,
...
+      '#default_value' => !empty($settings['css_id']) ? $settings['css_id'] : NULL,

defaults should be defined in block config somehow
and require config schema for this properties

Updated status per #6 and #7.

Add CSS style attribute

I've been looking at this to move it forward, and I'm questioning why 1/2 are necessary? The settings are attached to the source block configuration just like region in which if not set returns NULL as well as there is no needed update path as they will just return exactly that result which neither breaks nor changes existing variants.

As far as I can see, only tests need to be written here unless someone can enlighten me about the other 2 points

The patch at comment #11 works fine!
Someone knows if it will be merged to the next release?
Thanks

This seems to work nicely when placing blocks in the Panelizer UI. It would however be even better if we can use this in the Panels IPE form so it is configurable for all blocks placed through IPE.

Works fine for me as well. But integration with IPE will be better. Also there should be some instruction or help text e.g should that class name begin with dot or without dot etc.

#11 works

The patch from the #11 works for me too.
Was added warnings from #19
Please review

This patch works on its own, but conflicts with the patch provided here:

https://www.drupal.org/project/panels/issues/2769099#comment-12279981 - Block visibility conditions

Both provide functionality that was in D7 Panels but which is missing in the D8 version.

They are able to co-exist, but both try to add lines to the same function, public function submitForm(array &$form, FormStateInterface $form_state), in /src/Form/PanelsBlockConfigureFormBase.php.

I was able to manually add the failed lines from this patch into the function, below the lines added by the patch referenced above, and then both co-existed happily. For the record, those lines from this patch are:

$configuration['css_classes'] = $form_state->getValue('css_classes');
$configuration['css_id'] = $form_state->getValue('css_id');
$configuration['css_styles'] = $form_state->getValue('css_styles');

The final version of the function includes these lines:

$configuration = $this->block->getConfiguration();
$configuration['region'] = $form_state->getValue('region');
$configuration['visibility'] = $this->getVisibilityConditions()->getConfiguration();
$configuration['css_classes'] = $form_state->getValue('css_classes');
$configuration['css_id'] = $form_state->getValue('css_id');
$configuration['css_styles'] = $form_state->getValue('css_styles');
$this->getVariantPlugin()->updateBlock($this->block->getConfiguration()['uuid'], $configuration);

CSS injection has some security implications. Some kind of validation / filtering could be good at this. Anyway, the patch worked perfectly for me too in two different websites, thank you! Should be in RTBC but changing status to needs work due to pending tests.

Improve #21 patch and add full Panels IPE support with tests.
Please test and review.

I'm following up on #22 and #24.

In short, the patch in #24 works for me on Drupal 8.5.5, with Panels 8.x-4.3. I am now using it in 6 sites in 2 separate Drupal instances. Thanks for the efforts that have got us this far - are we RTBC?

As noted in #22, the patch in this issue clashes when you try to apply it after applying the latest patch here:

https://www.drupal.org/project/panels/issues/2769099#comment-12279981 - Block visibility conditions

However, if you install the Block Visibility patch before this one, it is then relatively simple to resolve the two failures that occur when applying this patch (and it is much easier to do it that way compared to applying this patch then the Block Visibility patch).

Essentially, the reason the second patch fails is that when it tries to insert new lines into one of the files being patched, it finds lines from the first patch that it doesn't recognise, so it fails. However, the lines in the two patches don't conflict - they can co-exist - so all you have to do is examine the .rej file after applying the second patch, and add those lines manually.

Thanks for the patch! Thought I'd help out with a test and review.

Cool to see tests included! The failed tests with 8.4 are because page manager no longer supports 8.4 IIUC. I think the failed tests with PHP 7.2 are not connected with this patch and are because \Drupal\Tests\panels\Unit\panels_ipe\RequestHandlerTestBase extends from a PHPUnit class directly rather than using Drupal's compatibility layer (and PHP 7.2 is tested with PHPUnit 6).

Manual testing

Standard

Update classes ✓
Update ID ✓
Update styles ✓
Sanitise input ✓

IPE

Update classes ✓
Update ID ✓
Update styles ✓
Preview block ✓
Sanitise input ✓

No issues I could find.

Code review

(I'm not super-familiar with Panels btw!)

1. It might be nice to consider how the functionality could be shared for use on non-Panel pages, but perhaps that's for another issue?

2. +++ b/config/schema/panels.schema.yml
   @@ -20,4 +20,13 @@ display_variant.plugin.panels_variant:
   +    css_classes:
   +      type: string
   +      label: The string of the block's classes
   

   Would it make sense for this to be an array of strings?

3. +++ b/config/schema/panels.schema.yml
   @@ -20,4 +20,13 @@ display_variant.plugin.panels_variant:
   +    css_styles:
   +      type: text
   +      label: The inline styles of the block
   

   text is translatable; I think it should be string like the others.

4. +++ b/config/schema/panels.schema.yml
   @@ -20,4 +20,13 @@ display_variant.plugin.panels_variant:
   \ No newline at end of file
   

   nitpick: missing newline

5. This is kindy nitpicky and could lead to bikeshedding, but personally I find the names like CSS ID, etc. a bit confusing: it's an HTML ID that might not be targeted by CSS. I'd go for more neutral terms both in code and the UI myself (just my two pennies!).

6. +++ b/panels_ipe/src/Form/PanelsIPEBlockPluginForm.php
   @@ -184,6 +185,29 @@ public function buildForm(array $form, FormStateInterface $form_state, $plugin_i
   +    $form['flipper']['front']['settings']['style_settings'] = [
   +      '#type' => 'details',
   +      '#title' => $this->t('Style settings'),
   +      '#open' => FALSE,
   +    ];
   +    $form['flipper']['front']['settings']['style_settings']['css_classes'] = [
   +      '#title' => $this->t('CSS classes'),
   +      '#type' => 'textfield',
   +      '#default_value' => !empty($block_config['css_classes']) ? $block_config['css_classes'] : NULL,
   +      '#description' => $this->t('Customize the style of the block by adding CSS classes. Separate multiple classes by spaces.'),
   +    ];
   +    $form['flipper']['front']['settings']['style_settings']['css_id'] = [
   +      '#title' => $this->t('CSS ID'),
   +      '#type' => 'textfield',
   +      '#default_value' => !empty($block_config['css_id']) ? $block_config['css_id'] : NULL,
   +      '#description' => $this->t('Customize the style of the block by adding CSS #id.'),
   +    ];
   +    $form['flipper']['front']['settings']['style_settings']['css_styles'] = [
   +      '#title' => $this->t('CSS styles'),
   +      '#type' => 'textarea',
   +      '#default_value' => !empty($block_config['css_styles']) ? $block_config['css_styles'] : NULL,
   +      '#description' => $this->t('Customize the style of the block by adding inline CSS.'),
   +    ];
   

   It might be worth trying to reuse the code from the main module rather than duplicating it in IPE. (Also could help with reuse in page manager.)

7. +++ b/panels_ipe/src/Form/PanelsIPEBlockPluginForm.php
   @@ -342,6 +383,23 @@ public function submitPreview(array &$form, FormStateInterface $form_state) {
        $build = $this->buildBlockInstance($block_instance, $this->panelsDisplay);
   

   Did you try to alter Drupal\panels_ipe\PanelsIPEBlockRendererTrait::buildBlockInstance() at all? It might be a better place to add the attributes (one central place). It seems that Drupal\panels_ipe\Controller\PanelsIPEPageController::getBlockModelData() also uses the result of buildBlockInstance() as part of the backbone app, but I'm not following the details.

8. +++ b/src/Plugin/DisplayVariant/PanelsDisplayVariant.php
   @@ -332,6 +334,30 @@ public function build() {
   +    foreach (Element::children($build) as $region_name) {
   +      foreach (Element::children($build[$region_name]) as $block_uuid) {
   +
   +        // Add CSS classes.
   +        $css_classes = !empty($this->configuration['blocks'][$block_uuid]['css_classes']) ? $this->configuration['blocks'][$block_uuid]['css_classes'] : '';
   +        $classes_array = explode(' ', $css_classes);
   +        foreach ($classes_array as $class) {
   +          $build[$region_name][$block_uuid]['#attributes']['class'][] = Html::cleanCssIdentifier($class);
   +        }
   +
   +        // Add CSS id.
   +        $css_id = !empty($this->configuration['blocks'][$block_uuid]['css_id']) ? $this->configuration['blocks'][$block_uuid]['css_id'] : '';
   +        if (!empty($css_id)) {
   +          $build[$region_name][$block_uuid]['#attributes']['id'] = Html::getId($css_id);
   +        }
   +
   +        // Add CSS styles.
   +        $css_styles = !empty($this->configuration['blocks'][$block_uuid]['css_styles']) ? $this->configuration['blocks'][$block_uuid]['css_styles'] : '';
   +        if (!empty($css_styles)) {
   +          $build[$region_name][$block_uuid]['#attributes']['style'] = $css_styles;
   +        }
   +      }
   +    }
   

   Instead of modifying all the blocks after they've been built, it might be easier to do it on a per-block basis in Drupal\panels\Plugin\DisplayBuilder\StandardDisplayBuilder?

9. +++ b/src/Plugin/DisplayVariant/PanelsDisplayVariant.php
   @@ -397,6 +423,9 @@ public function defaultConfiguration() {
   +      'css_classes' => '',
   +      'css_id' => '',
   +      'css_styles' => '',
   

   I think these are being added to the defaults for a panels variant plugin, not a block plugin.

10. +++ b/src/Plugin/DisplayVariant/PanelsDisplayVariant.php
    @@ -397,6 +423,9 @@ public function defaultConfiguration() {
    diff --git a/src/Tests/PanelsTest.php b/src/Tests/PanelsTest.php
    

    It might be nicer to add a new test rather than modifying \Drupal\panels\Tests\PanelsTest::testLayoutSettings().

11. nitpick: there's some occasions where you could use short array syntax to keep line lengths a little shorter.

Hope that helps, thanks again!

Edited: Add more context to point 3.

points 2 & 3 are debatable, sometimes it is very useful to have different classes for different languages

Thanks @andypost. I'd never considered using translatable classes. Unless I'm missing an implication, point 2 isn't really about translation (although it would change what the translation string would be if we did make it translatable, which might be what you're getting at?). I could've added more context for point 3: it's for the inline styling, which I doubt ever makes sense to be translatable? (Maybe I'm not being imaginative enough!)

@AndyF yep, I misread 2) - looks yep it could be a sequence but will require implode/explode processing so not sure if it's reasonable overhead

+++ b/src/Plugin/DisplayVariant/PanelsDisplayVariant.php
@@ -332,6 +334,30 @@ public function build() {
+        $classes_array = explode(' ', $css_classes);
+        foreach ($classes_array as $class) {
+          $build[$region_name][$block_uuid]['#attributes']['class'][] = Html::cleanCssIdentifier($class);
+        }

It's already being exploded, so I think if anything it'd be (a bit) more performant?

Patch updated and new feature: Custom styles for display variants.

Dependencies:
https://www.drupal.org/project/ctools/issues/3001713#comment-12786351

Test updated.

(coming soon)

Patch refactored. Please review and feedback.

Short array syntax.

Please fix project and issue metadata, wtf testing Drupal 8.4.x and PHP 5.5 :(
And related issue: https://www.drupal.org/project/panels/issues/3001708

Thanks for your efforts. The patch is working well for me in a Drupal 8.6.1 setup with PHP 7.1.

I just found a small nitpick: The patch reuses the same form field description for the block configuration and the panel variant configuration.

Customize the style of the block by adding CSS classes. Separate multiple classes by spaces.

In the Panel Variant configuration the first sentence is not accurate, because it's adding the css classes to the layout of the variant and not a block.

I just tested this patch, works! Thank you!

I agree with szeider #44, the wording should be more neutral or customized for the layout variant.

Maybe: "Customize the element style by adding CSS classes."

Ok, labels modified.

Seems the patch #46 works. Thanks!

#46 works

#46 works. Thanks!!!

#46 works great and is an extremely valuable enhancement to the functionality of the Panels module. Since Panels is mostly about layout, it seems a natural fit to be able to add css and other custom attributes in the ui.

Thanks for the patch.

The patch #46 looks good and applies successfully. +1 for RTBC.

Hi guys, are you planning to commit the #46?

#46 works perfectly for me on Drupal 8.6.10, php 7.2
Thanks

Test added with patch: #46

nevergone, you and everyone on this patch are awesome!

Since, Block Class module does not work with panels in drupal 8. This patch is a life saver...

#46 worked perfectly... I am a new-bee to Drupal 8 and git bash, so it took me awhile to figure out on how to apply this patch.

For other new-bees like myself, here is what I did to apply the patch in windows:

1) install git bash.
2) copy the #46 patch and put it into "modules/contrib/panels/(the patch name)"
3) open git bash and cd your way to panels.
4) type "ls" to list the files in the panels directory.
5) type "patch -p1 < (the patch file name)"
6) in drupal, "development/performance" clear the cache.
7) and in the varients/panels/content of your pages, in your blocks that you add or will add, you will see "style settings" to configure your class for the block.

Again, thank you so, so much everyone who contributed to this patch.

Oh, I forgot: Drupal 8.6.10, php 7.2, panels 8.4.3

Working fine. Thanks for the patch

The patch #46 works perfectly before enables layout builder.
#46 not working after enabled layout builder module. Screenshot attached.

Hi all,

If you are working with Layout Builder module mean, Please gothrough the issue, #2998114: Integration with Drupal core's new Layout Builder. It is working fine with layout buider.

Thanks

Patch #46 worked for me too, thank you! Would this patch be somehow used to add css element to variant body classes like we used to have it on Drupal 7?

The patch from #46 now fails to apply due to updates to the module.

I've re-rolled the patch and fixed to use the new tests directory.

It helps if you create the patch with all of the changes you made...

Hi,

I've migrated the panels_custom_attributes_in_panels_blocks-2849867-46.patch patch into the brach 8.x-4.x, and created this new version.

It only does not include the changes of files on /src/Tests, because the folder was removed.

PanelsTestTrait was removed in #63 but we still have references to it.

The attached patch removes those references and I hope it fixes test error.

Actually looks like the issue was precisely to remove PanelsTestTrait so adding it back.

There was a namespace error on previous patch but now I think generateCSSProperties() function should better reside in PanelsIPETestTrait as it is only used in panels_ipe test.

Added config schema fixes, test fix is still pending.

Attempting to fix test

Another fix for test + coding standard

The patch worked great, however it only allowed adding custom attributes to blocks, rather than regions too.

In order to style Regions as well, I switched to the Panels Style contrib module that adds plugin support, so that older D7 plugins could be ported and work with it as well.

Also the Panels Extra Styles D8 module uses the previous patch (#2296437: Implement style plugin type) that the module is based on for compatibility. I made both modules compatible with D9 and also added support for the Panels Style module (rather than the patch).

Here is a combination of everything I use to make this work:

Panels Style
#3031437: Error when region style set to default
#3062867: D8.5^/D9 compatibility - User temp store moved to core
#3179208: D8.7^/D9 compatibility - ConfigurablePluginInterface is deprecated
#3179124: D9 Version Requirements

Panels Extra Styles D8
#3179234: Use Panels Style contrib module as a dependency rather than Panels patch
#3179230: D9 Compatibility

The only thing missing from this setup, is being able to add custom attributes to variants. Hopefully this can be added at some point.

I hope this helps others as well!

Edit: I may have spoken too soon. I cannot save block configurations with this setup, only regions. I currently don't recommend this setup until this is fixed.

I really needed to add classes to a variant as @NWOM mentions above: "The only thing missing from this setup, is being able to add custom attributes to variants. Hopefully this can be added at some point." Instead of making this issue even longer I created a new issue and a patch if anyone is interested #3186390: Add default class and ability to configure additional classes to variants. Thank-you to everyone for helping bring back this awesome functionality.

@joel_osc: Sorry, I think my comment above may have been confusing. Basically the setup with the contrib modules patches (if they worked correctly) would provide classes for blocks and panes, but not variants. However the patch in this issue provides classes for blocks and variants, but not panes. I hope this clears that up :)

Thanks @NWOM for the clarification. I guess I will just keep watching this issue and the related issues and hope that panels soon returns to its former awesomeness. Cheers.

Wonder why this isn't included in the module itself but we're using a patch instead.

Hey guys,

I tried to apply 2 different patches for Panels and I'm getting an error.

Patch 1: [Block visbility conditions](https://www.drupal.org/project/panels/issues/2769099)

Patch 2: [Allow for block visibility rules](https://www.drupal.org/project/page_manager/issues/2858877)

`https://www.drupal.org/files/issues/2020-07-13/panels-block_visibility_c... (Block visibility conditions)`
`https://www.drupal.org/files/issues/2020-04-30/panels_custom_attributes_... (Custom attributes in panels blocks and variants)`
`Could not apply patch! Skipping. The error was: Cannot apply patch https://www.drupal.org/files/issues/2020-04-30/panels_custom_attributes_...`

I tried swapping them around and still the same error.

The weird part is that Both patches are somehow still applied and working.

*EDIT: The UI to add `style settings` in https://www.drupal.org/files/issues/2020-04-30/panels_custom_attributes_... is showing, but it actually doesn't add the CSS classes.

I tried following @millionleaves posts #22 and #27 and it's sort of working now. I'm able to apply style settings outside the block layout only. When I try adding style settings to individual blocks, it gets an error message.

Error message
Warning: implode(): Invalid arguments passed in Drupal\panels\Form\PanelsBlockConfigureFormBase->getCssStyleForm() (line 32 of modules\contrib\panels\src\Form\PanelsStyleTrait.php).
Drupal\panels\Form\PanelsBlockConfigureFormBase->getCssStyleForm(Array) (Line: 163)
Drupal\panels\Form\PanelsBlockConfigureFormBase->buildForm(Array, Object, 'page_manager.page', 'node_view--node_view-panels_variant-1', '9c0b0933-619b-4a3b-8693-96ba1cf2dccc')
call_user_func_array(Array, Array) (Line: 532)
Drupal\Core\Form\FormBuilder->retrieveForm('panels_edit_block_form', Object) (Line: 278)
Drupal\Core\Form\FormBuilder->buildForm(Object, Object) (Line: 91)
Drupal\Core\Controller\FormController->getContentResult(Object, Object)
call_user_func_array(Array, Array) (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 573)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 124)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 151)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 68)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 57)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 106)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 85)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 50)
Drupal\ban\BanMiddleware->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 52)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 708)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

I found out that adding Style Settings with In-page editor works. But as soon as I open page_manager and edit through there, all the CSS classes are removed.

I figured it out!

After following @millionleaves #22 and realized that my .rej file is different from his. So I updated it with the latest patch.

Updated it with the following:

$configuration = $this->block->getConfiguration();
    $configuration['region'] = $form_state->getValue('region');
    $configuration['visibility'] = $this->getVisibilityConditions()->getConfiguration();
    $configuration['css_classes'] = preg_split('/\s+/', trim($form_state->getValue('css_classes')));
    $configuration['html_id'] = $form_state->getValue('html_id');
    $configuration['css_styles'] = $form_state->getValue('css_styles');
    $this->getVariantPlugin()->updateBlock($this->block->getConfiguration()['uuid'], $configuration);

After refreshing cache, It still wasn't working. Then, I realize that StandardDisplayBuilder.php inside /src/Plugin/DisplayBuilder also had a .rej file. I updated to the following:

if ($access->isAllowed()) {
       $configuration = $block->getConfiguration();
       $block_render_array = [
          '#theme' => 'block',
          '#attributes' => [],
          '#contextual_links' => [],
          '#weight' => $weight++,
          '#configuration' => $configuration,
          '#plugin_id' => $block->getPluginId(),
          '#base_plugin_id' => $block->getBaseId(),
          '#derivative_plugin_id' => $block->getDerivativeId(),
          ];

Also keep in mind you need to apply this patch "https://www.drupal.org/project/page_manager/issues/2858877" in order to get "Block Visbility Conditions" to work.

Hope this helps someone who is trying to use "Block Visbility Conditions" and this patch for Page_manager and Panels.

@Sseto: Here is a patch that already combines both patches in case you want to add it to composer: #3183258: Block visibility conditions and Custom attributes combined patch for composer.

As a summary, the patch in this issue works, however it is incompatible with the Block Visibility patch, until one gets committed and the other patch gets re-rolled. As a workaround, the combined patch that I created can be used.

Reroll of patch #78 against current dev.

#89 now causes the following warning
Warning: Invalid argument supplied for foreach() in Drupal\panels\Plugin\DisplayBuilder\StandardDisplayBuilder->buildRegions() (line 165 of modules/contrib/panels/src/Plugin/DisplayBuilder/StandardDisplayBuilder.php).
will try to adjust that patch to fix this

Forgot to add the PanelsStyleTrait

No idea why that test fails

New patches are not compatible with old ones because of schema changes. Updating 21 patch to be compatible with latest version because it works for me.

RTBC patch from comment #94 labelled as patch 92 as it has tests, functionality works, great job! This is definately needed and should go in asap.

Patch #96 has one additional fail and is based on a much earlier implementation and lacks tests. Lets go with the patch from comment #94 please as the patch from #94 has tests and it works flawlessly.

Just to be clear, this is the patch I am setting as RTBC: https://www.drupal.org/files/issues/2023-01-11/panels_custom_attributes_...

Ok I committed #94 thanks all contributors, I tried to give credit where due and gave authorship to @nevergone for the tests because they tend to be the trickiest to get help with but are the real reason I'm committing this.

@eclipsegc mentioned (on slack) there are likely other modules out there that could accomplish this feature but bringing the expected features from previous versions seems reasonable.