Thank You for great module Facets!

I use it in own project, during setup found: When any facet displayed on homepage "is-active" css class disappeared from "Main navigation" (main) menu "Home" link. How to reproduce:

  1. Add "Main navigation" menu block (with available "Home" link) to display on site main page (if not added yet)
  2. Install: search_api, facets modules
  3. Create several nodes for indexing
  4. Create Search API DB server
  5. Create Search API index (use Search API DB server created above)
  6. Setup index for indexing nodes (that was added above) and index it
  7. Create views page (for example /articles) that use Search API index as data source
  8. On "Basic site settings" page set /articles as "Default front page"
  9. Visit site main page:
    • Indexed nodes displayed
    • "is-active" class available on "Home" link "Main navigation" menu
  10. Create any facet for Search API index created above
  11. Add created facet for display on site main page
  12. Visit site main page:
    • Indexed nodes displayed
    • Facet displayed
    • "is-active" class not appears on "Home" link "Main navigation" menu

Issue is in QueryString URL processor plugin, method buildUrls() code:

$get_params->set('page', $current_page);

This code not check is $current_page exists and add "page" param to ParameterBag.
Means (in my case):

  • User visit main page, URL is: /
  • $_GET["page"] param not exists because pagination not used yet
  • $current_page = NULL because $_GET["page"] param not exists
  • Code $get_params->set('page', $current_page); add "page" => NULL to current page request query ParameterBag
  • In system.module system_js_settings_alter() function Drupal 8 build and set path settings for drupalSettings.path (current page path information in JS). In my case it looks like:
      baseUrl: "/"
      currentLanguage: "en"
      currentPath: "articles"
      currentPathIsAdmin: false
      currentQuery: { page: null }
      isFront: true
      pathPrefix: "en/"
      scriptPath: null
  • /core/misc/active-link.js file use drupalSettings.path for build correct selectors for select currently "active" links and add "is-active" css class to it
  • Because drupalSettings.path.currentQuery contains not existed { page: null } param /core/misc/active-link.js build incorrect selectors
  • From this reason "Home" link is not selected and "is-active" css class not set

Attached patch fix this issue.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Antonnavi created an issue. See original summary.

Status: Needs review » Needs work

The last submitted patch, facets_fix_is_active_home_css_class.patch, failed testing.

The last submitted patch, facets_fix_is_active_home_css_class.patch, failed testing.

andypost’s picture

Looks that breaks a lot of tests, probably that's wrong

andypost’s picture

Status: Needs work » Needs review
FileSize
1.06 KB

I think this is a proper fix

Status: Needs review » Needs work

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

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

andypost’s picture

Status: Needs work » Needs review

Testing broken somehow, as example docs only patch in #2783859: Return value of ResultInterface::getChildren() is array

borisson_’s picture

Tests broke because #2777483: Unmet dependencies in search api, we can probaby fix that the same way as you fixed #2775963: FacetsSerializer views style plugin depends on optional rest module?

andypost’s picture

Yep, it makes sense for tests also

  • borisson_ committed d5c0117 on 8.x-1.x authored by andypost
    Issue #2783847 by andypost, Antonnavi: Broken is-active css class on...
borisson_’s picture

Status: Needs review » Fixed

Committed, thanks.

Status: Fixed » Closed (fixed)

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