As far as I can tell, there is no way to set the title of a facet.

In facets-item-list.html.twig:

- title: The title of the list.

{%- if title is not empty -%}
  <h3>{{ title }}</h3>
{%- endif -%}

This title is always empty, and I can see no way to set it in the CMS.

Additionally, I can't preprocess it to set the title variable, because in template_preprocess_facets_item_list() there is no way to tell which facet I am looking at. So I can't fetch the administrative title, or give it a title according to facet ID.

Both theme hooks should pass along some kind of context so that the preprocess functions and templates are able to do different things to different facets.

Example:

function facets_theme($existing, $type, $theme, $path) {
  return [
    'facets_result_item' => [
      'variables' => [
        'facet_id' => '', // add this
        'value' => '',
        'show_count' => FALSE,
        'count' => NULL,
        'is_active' => FALSE,
      ],
    ],
    'facets_item_list' => [
      'variables' => [
        'facet_id' => '', // add this
        'items' => [],
        'title' => '',
        'list_type' => 'ul',
        'wrapper_attributes' => [],
        'attributes' => [],
        'empty' => NULL,
        'context' => [],
      ],
    ],
  ];
}

That way I can load the Facet entity in preprocess to inspect it, or I can have a switch() statement based on facet ID.

CommentFileSizeAuthor
#14 interdiff-2877634.txt551 bytesborisson_
#14 2877634.patch4.24 KBborisson_
#12 2877634.patch3.7 KBborisson_
#12 interdiff-2877634.txt1.07 KBborisson_
#11 2877634.patch2.63 KBborisson_
#11 interdiff-2877634.txt2.66 KBborisson_
#6 the_title_variable_is-2877634-6.patch461 bytesborisson_
#5 facets-title_variable_not_set-2877634-5.patch417 bytesandreasderijcke
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Rob230 created an issue. See original summary.

Rob230’s picture

Rob230 CreditAttribution: Rob230 commented

Ah I see that the facet ID is in the attributes array, so that at least solves that part of my problem.

[
  '#attributes' => [
    'data-drupal-facet-id' => $facet->id(),
    'data-drupal-facet-alias' => $facet->getUrlAlias(),
  ],
]
borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
CreditAttribution: borisson_ as a volunteer and at Dazzle commented

Related to #2855320: Provide facet id and raw value in Facets summary result items but not the same.

I hadn't looked at this, because I tend to use the block title instead for adding titles.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
CreditAttribution: borisson_ as a volunteer and at Dazzle commented

Actually fixed with #2855320: Provide facet id and raw value in Facets summary result items's latest patch, that passes the facet in, I think that means we can close this issue.

andreasderijcke’s picture

andreasderijcke
Location Antwerpen / Gent
CreditAttribution: andreasderijcke at Calibrate commented
FileSize
facets-title_variable_not_set-2877634-5.patch417 bytes

I do not agree.

On multi language site, you can translate the facet name/title on the facet, which makes sense.
It doesn't make sense you have to translate the facet block to have the facet title translated on the page.

This should be a matter of setting the correct value for 'title', as it's all there in the twig.
Patch attached does the trick for me, without the need for additional preprocessing or twig overrides.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
CreditAttribution: borisson_ as a volunteer and at Dazzle commented
Status: Active » Needs review
FileSize
the_title_variable_is-2877634-6.patch461 bytes

Forgot to apply the original patch, so no interdiff, but the patch is small enough.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
CreditAttribution: borisson_ as a volunteer and at Dazzle commented
Status: Needs review » Reviewed & tested by the community

I'm very anxious to commit this, as it might break exisiting layouts, but it is correct and the previous behavior wasn't. I have tweeted to gather feedback: https://twitter.com/Borisson/status/964790347363741696

remydenton’s picture

remydenton CreditAttribution: remydenton at Pegasystems commented

The way I see it, existing users will fall into one of these categories:

  1. They didn't need the titles, so did nothing about the fact they were missing (seems like a silly use case, but it was the default, so there must be some).
  2. They worked around the missing titles by overriding the facets-item-list.html.twig template and printing it some other way (replacing title with facet.label, perhaps).
  3. They worked around the issue by printing the titles outside of the facets-item-list.html.twig template (in some wrapper, for example).
  4. They worked around the issue by setting the value of title in custom preprocessing without overriding facets-item-list.html.twig.

I can't think of an elegant way to handle this that wouldn't potentially have unintended changes for at least one of the four groups above. With the current patch, the only side affect I see is that groups 1 and 3 may get a title that they didn't want (most of group 1 probably did want the titles and just didn't know it, while group 3 was doing something at least a little unorthodox).

It's not ideal, but also not the end of the world. If it were me, I would commit it, mention it in the release notes, and remind anyone who complains that the module is still in beta :)

remydenton’s picture

remydenton CreditAttribution: remydenton at Pegasystems commented

Then again, in comment #3, borisson_ said:

I tend to use the block title instead for adding titles

Hardly "unorthodox" as I mentioned in my last comment, and probably something a lot of people might do, especially as site builders. In that case, my last comment might be too presumptuous as it would break things for all those folks...

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
CreditAttribution: borisson_ as a volunteer and commented
Status: Reviewed & tested by the community » Needs work

Discussed this at the belgian user group sprint just now with @swentel, @andreasderijcke and @StryKaizer. We're going to make this configurable to make sure that not all sites break. Going to do this now.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
CreditAttribution: borisson_ as a volunteer and commented
Status: Needs work » Needs review
Issue tags: +DUGBE
FileSize
interdiff-2877634.txt2.66 KB
2877634.patch2.63 KB

As discussed.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
CreditAttribution: borisson_ as a volunteer and commented
FileSize
interdiff-2877634.txt1.07 KB
2877634.patch3.7 KB

Now with test-coverage.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
CreditAttribution: borisson_ as a volunteer and commented
Status: Needs review » Needs work

NW based on the testfails.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
CreditAttribution: borisson_ as a volunteer and commented
Status: Needs work » Needs review
Issue tags: +DevDaysLisboa
FileSize
2877634.patch4.24 KB
interdiff-2877634.txt551 bytes

This should fix the test.

Nick_vh credited undefined.

Nick_vh’s picture

Nick_vh
he/him
Location Ghent
CreditAttribution: Nick_vh at Dropsolid commented
Status: Needs review » Fixed

  • Nick_vh committed 31eb067 on 8.x-1.x authored by borisson_ 
    Issue #2877634 by borisson_, andreasderijcke, swentel: The title...

Status: Fixed » Closed (fixed)

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

borisson_ credited swentel.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
CreditAttribution: borisson_ as a volunteer and commented

Nick_vh credited undefined.

That should've been swentel. Fixing that.