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
Avoid API request for API.getPagesComparisonsDisabledFor on Login pages #20578
Conversation
document.addEventListener('DOMContentLoaded', () => { | ||
this.loadComparisonsDisabledFor(); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change was needed as window.broadcast.isLoginPage()
checks if the body element has a certain id. But the javascript might currently already be parsed before the browser starts to render the html, which makes it impossible to look for html elements id.
Not sure if there is a smarter approach for this...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just a thought that came up, but an alternative might be to make Piwik::getLoginPluginName()
a global in jsGlobalVariables.twig and check if the URL module == that or 'Login'
haven't tested it, but the code looks fine |
This issue is in "needs review" but there has been no activity for 7 days. ping @matomo-org/core-reviewers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This worked well for me when testing, the unnecessary API call is not made when loading the login page 👍
- Functional review done
- Potential edge cases thought about (behavior of the code with strange input, with strange internal state or possible interactions with other Matomo subsystems)
- Usability review done (is anything maybe unclear or think about anything that would cause people to reach out to support)
- Security review done
- n/a Wording review done
- Code review done
- n/a Tests were added if useful/possible
- none Reviewed for breaking changes
- n/a Developer changelog updated if needed
- n/a Documentation added if needed
- n/a Existing documentation updated if needed
Description:
While testing another issue I saw that on login pages a request to
API.getPagesComparisonsDisabledFor
was sent. This is actually unneeded.Review