Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Vue] fix url location updating regression in MatomoUrl.updateLocation use #18449

Merged
merged 5 commits into from Dec 7, 2021

Conversation

diosmosis
Copy link
Member

Description:

To reproduce the issue on 4.x-dev, either change the period in the period selector (eg, select a month from a day) or try to compare a segment. There are some URL params that cannot appear in the hash, eg module/action, so the base set of parameters when changing the URL has to be determined carefully.

Review

@diosmosis diosmosis added Needs Review PRs that need a code review Regression Indicates a feature used to work in a certain way but it no longer does even though it should. labels Dec 4, 2021
@diosmosis diosmosis added this to the 4.7.0 milestone Dec 4, 2021
@diosmosis diosmosis force-pushed the vue-period-selector-regression branch from 00529d2 to 90ef0e8 Compare December 4, 2021 11:25
@diosmosis diosmosis requested a review from sgiehl December 6, 2021 15:43
Copy link
Member

@sgiehl sgiehl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally. Was able to reproduce the problem and can confirm the fix seems to work.

@sgiehl sgiehl removed the Needs Review PRs that need a code review label Dec 7, 2021
@sgiehl sgiehl merged commit 92ed7f8 into 4.x-dev Dec 7, 2021
@sgiehl sgiehl deleted the vue-period-selector-regression branch December 7, 2021 09:08
@meyer-net

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Regression Indicates a feature used to work in a certain way but it no longer does even though it should.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants