Problem/Motivation

When no facets are active and 'Adds reset facets link.' processor is selected, the reset facets link is still shown. This is confusing for users.

Proposed resolution

Only show the link if there is something to reset.

CommentFileSizeAuthor
#7 facets_rerolled-patch_2960137_7.patch3.72 KBStefdewa
#4 2960137.patch3.73 KBborisson_
#4 interdiff-2960137.txt2.79 KBborisson_
#2 facets-dont-show-reset-2960137-2.patch2.04 KBSutharsan
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Sutharsan created an issue. See original summary.

Sutharsan’s picture

Sutharsan CreditAttribution: Sutharsan at LimoenGroen commented
Status: Active » Needs work
FileSize
facets-dont-show-reset-2960137-2.patch2.04 KB
borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
CreditAttribution: borisson_ commented

This makes a lot of sense, we should probably create a regression test here as well. I think that the changes to twig file here are an accident?

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
CreditAttribution: borisson_ commented
Status: Needs work » Needs review
FileSize
interdiff-2960137.txt2.79 KB
2960137.patch3.73 KB
Sutharsan’s picture

Sutharsan CreditAttribution: Sutharsan at LimoenGroen commented
Status: Needs review » Reviewed & tested by the community

I think that the changes to twig file here are an accident?

Yes.

The test looks good, thanks.

borisson_’s picture

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

This patch no longer applies, it should be rerolled.

Stefdewa’s picture

Stefdewa CreditAttribution: Stefdewa at Calibrate for Aquarius Age Company commented
Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
facets_rerolled-patch_2960137_7.patch3.72 KB

Rerolled.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
CreditAttribution: borisson_ commented
Status: Needs review » Reviewed & tested by the community

Looks great.

  • Nick_vh committed b2eeb56 on 8.x-1.x authored by Stefdewa 
    Issue #2960137 by borisson_, Sutharsan, Stefdewa: Only show reset link...
Nick_vh’s picture

Nick_vh
he/him
Location Ghent
CreditAttribution: Nick_vh at Dropsolid commented
Status: Reviewed & tested by the community » Fixed

Thanks Stefdewa & Sutharsan!

Status: Fixed » Closed (fixed)

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

vlad.dancer’s picture

vlad.dancer
Primary language Ukrainian
Location Kyiv
CreditAttribution: vlad.dancer as a volunteer and at Drupal Ukraine Community for Xspace commented

Well, despite of the fact that it is reasonable from the first look, I add here one more use case.
Let's imagine that we don't want autosubmit facets (https://www.drupal.org/project/facets/issues/3172373) then some facets options combinations could produce zero results. And then it is confusing for users getting no reset link :)
Maybe we need to add or condition for checking empty result set?

  public function build(FacetsSummaryInterface $facets_summary, array $build, array $facets) {
    // ....
    $count = $facets_summary->getFacetSource()->getCount();

    // Do nothing if there are no selected facets.
    if (empty($build['#items']) && $count > 0) {
      return $build;
    }
    // ....
    if (!$hasReset || $count > 0) {
      return $build;
    }
    // ....