As the different widgets look completely different, and themers may want to style them differently, it would be nice to have template suggestions based on widget type.

While we're at it, the ability to override for a single facet would probably be useful too.

So something like:

- facets-item-list--WIDGET_TYPE--FACET_ID
- facets-item-list--WIDGET_TYPE
- facets-item-list

CommentFileSizeAuthor
#9 2896202-9.facets.add-template-suggestions-for-facetsitemlisthtmltwig-based-on-widget-type-and-facet-ID.patch4.74 KBjoachim
#6 interdiff.2896202.4-6.txt4.6 KBjoachim
#6 2896202-6.facets.add-template-suggestions-for-facetsitemlisthtmltwig-based-on-widget-type-and-facet-ID.patch4.98 KBjoachim
#4 2896202-4.facets.add-template-suggestions-for-facetsitemlisthtmltwig-based-on-widget-type-and-facet-ID.patch1.72 KBjoachim
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

joachim created an issue. See original summary.

borisson_’s picture

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

That's a solid idea.

joachim’s picture

joachim CreditAttribution: joachim at Torchbox commented
Assigned: Unassigned » joachim

Patch coming.

joachim’s picture

joachim CreditAttribution: joachim at Torchbox commented
Status: Active » Needs review
FileSize
2896202-4.facets.add-template-suggestions-for-facetsitemlisthtmltwig-based-on-widget-type-and-facet-ID.patch1.72 KB

Here's the patch.

I've added a helper method to the widget base class, to prevent repetition in the date widget (and other widgets to come...).

There's one more instance of '#theme' => 'facets_item_list' in buildListItems(), which I'm not sure about. It appears to be for nested lists, so I don't know if it makes sense to have the suggestion there too -- I don't know enough about this to be sure.
Also, buildListItems() doesn't have the $facet available, so to add them here we'd need to change its signature to get the $facet object passed in.

Happy to reroll if you want it adding there too.

borisson_’s picture

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

I'm not sure if it's needed inside the buildListItems, for consistency I'd say we should.

joachim’s picture

joachim CreditAttribution: joachim at Torchbox commented
Status: Needs work » Needs review
FileSize
2896202-6.facets.add-template-suggestions-for-facetsitemlisthtmltwig-based-on-widget-type-and-facet-ID.patch4.98 KB
interdiff.2896202.4-6.txt4.6 KB

Updated patch.

Changes since the last patch:
- docs
- fix semicolon
- Added facet param to build item list
- added theme suggestions to build item list
- fixed closure
Patch and interdiff created by Dorgflow.

I've filed #2896412: ArrayWidget::buildListItems() should use inheritdoc tag in passing, and added the parameter to the repeated docs in ArrayWidget rather than fix that here.

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

Looks great

borisson_’s picture

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

Patch doesn't apply anymore.

joachim’s picture

joachim CreditAttribution: joachim at Torchbox commented
Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
2896202-9.facets.add-template-suggestions-for-facetsitemlisthtmltwig-based-on-widget-type-and-facet-ID.patch4.74 KB

Here's the patch from my rebased feature branch.

borisson_’s picture

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

Committed and pushed, thanks!

  • borisson_ committed 9064a63 on 8.x-1.x authored by joachim 
    Issue #2896202 by joachim: add template suggestions for facets-item-list...

Status: Fixed » Closed (fixed)

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