localgovdrupal / localgov_base

The base theme for LocalGov Drupal websites.
8 stars 13 forks source link

System messages are no longer displaying #154

Closed andybroomfield closed 2 years ago

andybroomfield commented 2 years ago

Using Localgov_base and themes that derive from it. Even though the messages block is placed in the messages region, and this is being rendered in the theme, the messages themselves are not being rendered.

Tested by doing a flush cache which should set a status message. This works in Bartick and Localgov_theme. Does not show in localgov_base and child theme.

andybroomfield commented 2 years ago

Its this section in localgov_base.theme.

foreach ($system_region as $key => $value) {
    $region = $key;
    $has = 'has_' . $region;
    $copy = $variables['page'][$region];
    $rendered = Drupal::service('renderer')->renderPlain($copy);
    $variables[$has] =  (!empty(trim(strip_tags($rendered)))) ? true : false;
  }

Removing it brings the messges back (have to force set the has_messages etc variable though). Is this then marking the section as rendered already so its not triggered, even though it is a copy?

andybroomfield commented 2 years ago

Removing the copy variable seems to solve this. Changing

 $rendered = Drupal::service('renderer')->renderPlain($copy);

to

 $rendered = Drupal::service('renderer')->renderPlain($variables['page'][$region]);

Would this likly effect anything else?

andybroomfield commented 2 years ago

Applying the above fix however breaks the map view on a directory channel. I guess that is the reason to render it in a copy?

markconroy commented 2 years ago

I wonder why this affects the messages block, but not other blocks? And I wonder why your line change fixes the messages block, but breaks (at least) one other block?

andybroomfield commented 2 years ago

@markconroy My guess is that the messages block renders a placeholder, so rending the copy might be causing the placeholder for the messages to generate be put into the copy so it won't show. Whereas rending the region itself might be adding in some extra libraries that when re-rendered arn't being attached, as its javascript elements like maps on directories that break.

finnlewis commented 2 years ago

Discussing in 'merge monday'. Interesting! @stephen-cox and @finnlewis hope to find some time to investigate this also.

markconroy commented 2 years ago

I've made a change here to:

  1. Add an array of "special" regions such as messages (currently only messages region is in the array)
  2. If an item is in this array, we render it, but not via $copy
  3. Added the messages.css file to the global library (it wasn't getting loaded via the template, I think because the template in this case renders too late)

PR #162

andybroomfield commented 2 years ago

Can consider fixed?

stephen-cox commented 2 years ago

Yep - this was fixed #162