Active language in language switcher does not have 'is-active' class if query string present in URL, e.g. pager ?page=0.

Comments

matulis created an issue. See original summary.

matulis’s picture

Title: No active language if pager used on a views page » No active language in language switcher for URL with query string
Issue summary: View changes
Issue tags: -views
matulis’s picture

Version: 8.1.1 » 8.1.7
rodrigoaguilera’s picture

Version: 8.1.7 » 8.2.x-dev

The fix should be for the latest stable branch.

This happens also in views with exposed filters that add parameters to the query

dpacassi’s picture

Status: Active » Needs review
Issue tags: +JavaScript, +Dublin2016
FileSize
697 bytes

I was able to reproduce the bug on Drupal 8.2.x-dev with the standard installation profile.
I've tracked down the error to the core's active-link.js file, which set's the is-active class for active links.

Currently, the JS only adds the is-active class if the URL in the link matches the current URL exactly (including URL parameters).
While that's for most links intended, language links should always be marked as is-active as long as they match the current language.

I've written a patch which covers this issue, feel free to review it.

nod_’s picture

Does the same issue happens when the user is anonymous?

There are 2 different script checking for the active link, one in JS and one in PHP. If the JS is bugged, the PHP most likely is too.

dpacassi’s picture

Status: Needs review » Needs work

Oh, nice input, I didn't know about the PHP part.
The issue still exists for anonymous users, I'll try to fix the PHP part as well.

dpacassi’s picture

Status: Needs work » Needs review
FileSize
2.07 KB
1.25 KB

Fixed the PHP part as well, see the attached patch and interdiff.

SteffenR’s picture

Assigned: Unassigned » SteffenR

I'll have a look at this issue / review it.

SteffenR’s picture

Assigned: SteffenR » Unassigned
Status: Needs review » Reviewed & tested by the community
FileSize
346.68 KB

I could reproduce the bug. After adding the patch i get the is-active class for anonymous and logged in users as well.
From my perspective it looks fine and could be set to RTBC.

Attached you find a screenshot, showing that its working like it should.

SteffenR

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 8: active-language-links-2775651-8.patch, failed testing.

SteffenR’s picture

Status: Needs work » Reviewed & tested by the community

Not sure why testbot resetted the status of the patch to "needs work" - all tests are green.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 8: active-language-links-2775651-8.patch, failed testing.

droplet’s picture

This isn't a bug but a new feature IMO. Or say this isn't a bug of active-link.js but the language switcher (or language system). There missing query string in language switcher link.

If page=1 changing the page content, language switcher redirects you to another content.

rodrigoaguilera’s picture

@droplet that is a good point.

If the language switcher renders the links with the current query parameters it will be marked as active and it will take you to a more "proper" page when you click and switch languages.

I think the patch should be reworked so it does that.

I still feel like it is a bug since having query parameters shouldn't make the user lose the language context like filtering a view or navigate to a different page

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

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should 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.