Problem/Motivation

In D7 Facet API, there is an option on widgets that allows site builders to configure a soft limit. All facets above this limit are hidden and a clickable "Show more" link is shown.

Proposed resolution

Port the functionality to Facets (D8).

Remaining tasks

None.

User interface changes

"Show more"/"Show fewer" JS enabled links.

API changes

None.

Data model changes

None.

Comments

claudiu.cristea created an issue. See original summary.

claudiu.cristea’s picture

Title: Port "Show more"/"Show fewer" functionality from D7 » Port "Show more"/"Show less" functionality from D7
Status: Active » Needs review
FileSize
7.12 KB

Patch.

Status: Needs review » Needs work

The last submitted patch, 2: 2724381-2.patch, failed testing.

The last submitted patch, 2: 2724381-2.patch, failed testing.

claudiu.cristea’s picture

Status: Needs review » Needs work

The last submitted patch, 5: 2724381-5.patch, failed testing.

The last submitted patch, 5: 2724381-5.patch, failed testing.

claudiu.cristea’s picture

Sorry, the patch from #5 was malformed. interdiff from #5 is OK.

claudiu.cristea’s picture

Issue tags: +Needs manual testing
StryKaizer’s picture

+++ b/src/Plugin/facets/widget/LinksWidget.php
@@ -61,6 +62,12 @@ class LinksWidget implements WidgetInterface {
+    if (!empty($configuration['soft_limit'])) {
+      $build['#attached']['library'][] = 'facets/soft-limit';
+      $build['#attached']['drupalSettings']['facets']['softLimit'][$facet->id()] = (int) $configuration['soft_limit'];
+    }

Is this only empty when soft_limit is enabled?
No need to add the js library if its disabled I suppose

claudiu.cristea’s picture

@StryKaizer, yes this is what I'm doing here. I'm adding the library only when soft_limit is not 0 (aka "No limit").

StryKaizer’s picture

Okay, ignore my comment
I never use the empty command obviously ;)

Patch looks good.
I used data-attribute instead in the checkbox/dropdown widgets for passing on config to js, but we prolly should use drupal.settings as you did in this patch for more consistency. I'll create an issue for that.

Code is rtbc for me, but still needs manual testing.

claudiu.cristea’s picture

I changed the your checkbox data-facet-id > data-drupal-facet-item-id because it's about items. The I added a new data-drupal-facet-id to the list of facets (the UL list). Probably we should keep them.

claudiu.cristea’s picture

Probably I would move the data-drupal-facet-item-id from checkbox to its parent (Links) to have it in all situations.

StryKaizer’s picture

Status: Needs review » Needs work

patch from #8 was not working with me (both for lists and checkbox widget)
It looks like this is a js selector issue, as you already mentioned in #14

data-drupal-facet-id is on the parent div in my case, instead of the ul

claudiu.cristea’s picture

Works for me. I tried Links, Checkbox and a custom widget that is extending Checkbox. But my site the heavily themed so, probably, this affects where the selector is placed?

claudiu.cristea’s picture

data-drupal-facet-id is on the parent div in my case, instead of the ul

Can you investigate why this happens? I cannot reproduce that situation

StryKaizer’s picture

Sure, doing right now.

Vanilla drupal with standard profile (so bartik) here.
Drupal 8.2.x

claudiu.cristea’s picture

OK, we are fixing also a bug here (see the interdiff.txt). The block build should be an array of items. Otherwise \Drupal\block\BlockViewBuilder::preRender() is moving all #attributes on the block level.

Status: Needs review » Needs work

The last submitted patch, 19: 2724381-19.patch, failed testing.

The last submitted patch, 19: 2724381-19.patch, failed testing.

claudiu.cristea’s picture

Yeah, forgot to treat also the empty scenario.

StryKaizer’s picture

Great work, nice clean code!
Tested, works as expected.

Extremely minor nitpick, but this can also be fixxed while commiting I suppose.

RTBC for me

  1. +++ b/src/Plugin/facets/widget/LinksWidget.php
    @@ -155,20 +162,30 @@ class LinksWidget implements WidgetInterface {
    +   * @todo This is inheriting noting. We need a method on the interface and,
    

    Extremely minor nitpick: Typo in todo ;) (not(h)ing)

  2. +++ b/src/Plugin/facets/widget/LinksWidget.php
    @@ -155,20 +162,30 @@ class LinksWidget implements WidgetInterface {
    +    // @todo This should be handled upstream, in facet entity. Facet schema
    +    //   should be fixed and all configs should get sane defaults.
    

    Created an issue for this #2725453: Refactor widget plugins by adding interface, base class, schema

StryKaizer’s picture

Status: Needs review » Fixed
StryKaizer’s picture

Dropdown got broken by this, fix coming up

  • StryKaizer committed dd6660d on 8.x-1.x
    Issue #2724381 by StryKaizer: Port "Show more"/"Show less" functionality...

Status: Fixed » Closed (fixed)

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