When using #theme => links, you can set the 'set_active_class' option and get the CSS class is-active added to both the LI and the A element when the link's URL matches the current URL.

However, if the URL in question contains a query, the is-active class only gets added to the A element, and not the LI.

This is because template_preprocess_links() is not correctly getting the query parameters from the URL:

          // Add a "data-drupal-link-query" attribute to let the
          // drupal.active-link library know the query in a standardized manner.
          if (!empty($link['query'])) {
            $query = $link['query'];
            ksort($query);
            $li_attributes['data-drupal-link-query'] = Json::encode($query);
          }

The query is no longer in $variables['links']['query'], but part of the URL object passed in $variables['links']['url'].

CommentFileSizeAuthor
#28 2726471-28.patch1.84 KBduaelfr
#21 Screenshot 2020-07-28 at 8.21.43 AM.png33.84 KBsamiullah
#21 Screenshot 2020-07-28 at 8.18.06 AM.png126.02 KBsamiullah
#20 interdiff_17-20.txt987 bytesvsujeetkumar
#20 2726471_20.patch1.84 KBvsujeetkumar
#17 2726471_17.patch745 bytesvsujeetkumar
#3 2726471-3.drupal.links-active-class-with-query.patch658 bytesjoachim
#2 2726471-2.drupal.links-active-class-with-query.patch645 bytesjoachim

Comments

joachim created an issue. See original summary.

joachim’s picture

joachim commented
Status: Active » Needs review
StatusFileSize
new 2726471-2.drupal.links-active-class-with-query.patch645 bytes

The 'query' property is on the URL that's set in 'url', not on the link array itself.

joachim’s picture

joachim commented
Issue summary: View changes
StatusFileSize
new 2726471-3.drupal.links-active-class-with-query.patch658 bytes

Updated patch. Previous patch was still looking in $links['query'], which is obsolete.

zerolab’s picture

zerolab commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +dcbristol2016

Using this patch in production without any issue.
Code looks sound.

Thank you

xjm’s picture

xjm
she/her
Primary language English
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests

Thanks @zerolab and @joachim!

Looks like we could use some test coverage for this bug?

smaz’s picture

smaz
Primary language English
Location England, UK
commented

I assume the test will need to be in core/modules/system/src/Tests/Theme/FunctionsTest.php, as FunctionsTest::testLinks() & FunctionsTest::testIndexedKeyedLinks() check the results of links.html.twig.

If that's the case, and no one gets to this, I'll try and take a look.

Cheers

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

mpp’s picture

mpp commented

The patch doesn't solve the issue of links that aren't highlighted when there's a query parameter in the url of the current page.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

rgnyldz’s picture

rgnyldz commented

The patch on # is not applying to 8.9.2

The line is now on 730 in file /core/includes/theme.inc

And the issue is still here on D8

vsujeetkumar’s picture

vsujeetkumar commented
Status: Needs work » Needs review
StatusFileSize
new 2726471_17.patch745 bytes

Re-roll patch for 9.1.x, Please review.

rgnyldz’s picture

rgnyldz commented

Patch is applying for 8.9.2 but the issue still persists.

For the issues sake I'll test it on a clean D9 and get back with feedbacks soon.

Status: Needs review » Needs work

The last submitted patch, 17: 2726471_17.patch, failed testing. View results

vsujeetkumar’s picture

vsujeetkumar commented
Status: Needs work » Needs review
StatusFileSize
new 2726471_20.patch1.84 KB
new interdiff_17-20.txt987 bytes

@RgnYLDZ thanks for replying, Waiting for your feedback.

I also fixed one failed test, Please have a look and advise.

samiullah’s picture

samiullah commented
StatusFileSize
new Screenshot 2020-07-28 at 8.18.06 AM.png126.02 KB
new Screenshot 2020-07-28 at 8.21.43 AM.png33.84 KB

@vsujeetkumar
Applied the patch on the clean install of drupal 9.1.x
Was able to see active class appending to links with query

This can be moved for RTBC

Before: https://www.drupal.org/files/issues/2020-07-28/Screenshot%202020-07-28%2...

After: https://www.drupal.org/files/issues/2020-07-28/Screenshot%202020-07-28%2...

samiullah’s picture

samiullah commented
Status: Needs review » Reviewed & tested by the community
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Reviewed & tested by the community » Needs work

I'm wondering if the fact that we need to fix the assertion in the failing test means that something was broken because the test documents the following assumption:

    // Assert that the language switching block is displayed on the frontpage
    // and ensure that the active class is added when query params are present.

It would be good to also have explicit test coverage for this.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

gwvoigt’s picture

gwvoigt
Location London, ON 🇨🇦
commented

#20 fails on 9.1.6
Never mind, I guess my issue was not related to this one.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

duaelfr’s picture

duaelfr
he/him
Primary language French
Location Montpellier, France
commented
StatusFileSize
new 2726471-28.patch1.84 KB

Just a reroll for 9.4.x to see if it still works.

carsteng’s picture

carsteng commented
Related issues: +#2845319: The highlighting of the active links does not respect query strings and fragment identifiers

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

duaelfr’s picture

duaelfr
he/him
Primary language French
Location Montpellier, France
commented
Status: Needs work » Closed (duplicate)

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.