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

Fixed issue #17135: activating Public statistics option causes json to display #2545

Conversation

gabrieljenik
Copy link
Collaborator

The problem was that when the statistics page is loaded by PJAX the Chart.min.js file is not loaded. Then, when you go to the rendersidemenylink pages (also by PJAX), you click a code in statistics.js that depends on Chart, so the code that handles the form submission by AJAX does not get executed.

@Shnoulle
Copy link
Collaborator

LSYii_ClientScript::POS_BEGIN work with pajax ?
No issue in code.

@Shnoulle Shnoulle added Code review done Version checked for code issue without testing and removed Needs code review labels Jul 25, 2022
@gabrieljenik
Copy link
Collaborator Author

LSYii_ClientScript::POS_BEGIN work with pajax ?

Seems so. Not sure exactly why, but seems so :)

@Shnoulle
Copy link
Collaborator

CClientScript::POS_BEGIN : the script is inserted at the beginning of the body section.

pjax replace whole body ?

Must check with pos_END too

CClientScript::POS_END : the script is inserted at the end of the body section.

Only POS_HEAD and POS_LOAD and POS_READY didn't work directly then ?

@gabrieljenik
Copy link
Collaborator Author

pjax takes scripts from the following elements:
image

When a script is registered by default is sent to HEAD, so it is not caught by PJAX.

@Shnoulle
Copy link
Collaborator

But more : since it replace HTML : body content are done ther ? No ?

I mean : if pjax load


<div id="hideme">Hide me</div>
<script>$("#hideme").hide())</script>

It work.

The issue is more for scriptFile i think.

@gabrieljenik
Copy link
Collaborator Author

The issue is more for scriptFile i think.

I believe issue is related to scripts set with registerScript() and registerScriptFile()

@gabrieljenik gabrieljenik added Tested OK This PR has been tested by QA and works as expected and removed Needs testing minor impact Minor impact update / enhancement labels Jul 27, 2022
@olleharstedt olleharstedt merged commit b78af1d into LimeSurvey:master Aug 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code review done Version checked for code issue without testing Tested OK This PR has been tested by QA and works as expected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants