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

Fix setting with_imported to false explicitly in the URL #4148

Merged
merged 1 commit into from
May 27, 2024

Conversation

RobertJoonas
Copy link
Contributor

@RobertJoonas RobertJoonas commented May 27, 2024

Changes

Currently, when clicking on the imported exclude/include icon on the dashboard (see with-imported-switch.js), it doesn't set the with_imported to false due to PlausibleSearchParams class dropping any keys with false boolean value. The default for the with_imported field is true so we need to explicitly set it to false sometimes. Otherwise we'd end up with a URL identical to the current location, and the button simply doesn't work.

What I've done here is set the value as a string instead. Not aware of any other cases of this causing problems but maybe it would be worth to double-check (cc @macobo ). Is it necessary to drop the keys with false values or is it only an optimization?

@RobertJoonas RobertJoonas requested a review from macobo May 27, 2024 10:13
@macobo
Copy link
Contributor

macobo commented May 27, 2024

Right, this broke during refactoring. The dropping of flags behavior is intentional as it simplifies components reading/setting values significantly. I guess this flag behaves unconventionally as it doesn't only read from the URL.

As I don't have a great fix in mind for this of top of my head, let's go with this solution!

@zoldar zoldar merged commit 4d7a67d into master May 27, 2024
8 checks passed
@zoldar zoldar deleted the imported-switch-bug branch May 27, 2024 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants