backdrop-contrib / views_fieldsets

Creates fieldset (and details and div) in Views fields output, to group fields.
GNU General Public License v2.0
0 stars 2 forks source link

Only works with details - not with fieldset or div #3

Open argiepiano opened 2 months ago

argiepiano commented 2 months ago

I have a view and I want to wrap a Views Field View field with a collapsible fieldset:

This is my Views configuration:

Screen Shot 2024-08-31 at 12 55 26 PM Screen Shot 2024-08-31 at 12 55 34 PM
argiepiano commented 2 months ago

Checking the markup with the browser tools I see the legend is left empty, which prevents the fieldset from being ope/closed:

Screen Shot 2024-08-31 at 12 58 45 PM
argiepiano commented 2 months ago

Furthermore, if I choose <details><summary>, the summary HTML tag is never inserted, plus the details tag doesn't have the open attribute (despite the fact that I didn't choose "collapsed" in the UI), meaning that it will always be shown as collapsed regardless of what I pick in the UI.

Screen Shot 2024-08-31 at 1 04 22 PM
argiepiano commented 2 months ago

I've found the problem: the custom preprocess function _template_preprocess_views_fieldsets is never called.

This function should be added to the theme registry in views_fieldsets_theme_registry_alter(), but that actually never happens. I'm pasting the faulty function below. As you can see, $info is never assigned back to the registry.

function views_fieldsets_theme_registry_alter(&$registry) {
  foreach ($registry as $hook => $info) {
    // Splice in our generic preprocessor for all fieldset types.
    if (strpos($hook, 'views_fieldsets_') === 0 && isset($info['views_fieldsets_label'])) {
      // It must come directly after 'template_preprocess', or first if that doesn't exist.
      $index = array_search('template_preprocess', $info['preprocess functions']);
      if ($index !== FALSE) {
        array_splice($info['preprocess functions'], $index + 1, 0, '_template_preprocess_views_fieldsets');
      }
      else {
        array_unshift($info['preprocess functions'], '_template_preprocess_views_fieldsets');
      }
    }
    unset($info);
  }
}

the solution is to modify this function as follows:

function views_fieldsets_theme_registry_alter(&$registry) {
  foreach ($registry as $hook => &$info) {
    // Splice in our generic preprocessor for all fieldset types.
    if (strpos($hook, 'views_fieldsets_') === 0 && isset($info['views_fieldsets_label'])) {
      // It must come directly after 'template_preprocess', or first if that doesn't exist.
      $index = array_search('template_preprocess', $info['preprocess functions']);
      if ($index !== FALSE) {
        array_splice($info['preprocess functions'], $index + 1, 0, '_template_preprocess_views_fieldsets');
      }
      else {
        array_unshift($info['preprocess functions'], '_template_preprocess_views_fieldsets');
      }
    }
  }
}

I'll submit a PR.

argiepiano commented 2 months ago

PR #4 submitted.

argiepiano commented 2 months ago

NOTE: this PR does not fix the issue of the details element always appearing collapsed, regardless of the choice in the UI.

laryn commented 2 months ago

@argiepiano Thanks! I've stopped using Coder Upgrade as regularly when porting modules because at one point there was an issue where it removed & references -- looks like you've uncovered one of those. Sorry about that!

Since you've been in the code recently -- I see you're also removing the unset($info); but the D7 version contains that:

Can you explain briefly that change? I'll try to test this PR this week.

argiepiano commented 2 months ago

Hi @laryn. The unset($info) does not harm but it serves no purpose. The variable $info gets reassigned on the next loop iteration anyway, and unsetting doesn't do anything to the referenced array.