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

Fixes #24416: Elm properties app is loaded multiple times #5477

Conversation

clarktsiory
Copy link
Contributor

@clarktsiory clarktsiory commented Mar 15, 2024

https://issues.rudder.io/issues/24416

The fix consist of :

  • loading the script when the group page loads, or else it will be loaded multiple times
  • mounting and initializing an Elm app for node properties when the "Properties" tab content is shown
    So we just move everything related to the Elm app "earlier" in the HTML structure, from NodeGroupForm details to the main Group page.

Caveats of this is that a new Elm app will be initialized each time the "Properties" tab is shown, but changing the Elm model to re-render with every group change action is heavier (we would likely need to make Elm apps global variables, to call ports on every action from the Scala templates). We could do that in another PR.

@RaphaelGauthier
Copy link
Member

The error no longer appears when I click on different groups, but if I click several times on the Properties tab I get a new error:

On firefox :
errorjsfirefox

On chrome :
errorjschrome

@clarktsiory
Copy link
Contributor Author

PR updated with a new commit

@clarktsiory
Copy link
Contributor Author

@RaphaelGauthier The error is cryptic but obviously the tab is already mounted by the Elm app so the selector nodeproperties-app is empty 😁

@Normation-Quality-Assistant
Copy link
Contributor

This PR is not mergeable to upper versions.
Since it is "Ready for merge" you must merge it by yourself using the following command:
rudder-dev merge https://github.com/Normation/rudder/pull/5477
-- Your faithful QA
Kant merge: "To be is to do."
(https://ci.normation.com/jenkins/job/merge-accepted-pr/82419/console)

@clarktsiory
Copy link
Contributor Author

OK, squash merging this PR

@clarktsiory clarktsiory force-pushed the bug_24416/elm_properties_app_is_loaded_multiple_times branch from 78903b8 to ec6c5bf Compare March 28, 2024 11:20
@clarktsiory clarktsiory merged commit fe93139 into Normation:branches/rudder/8.0 Mar 28, 2024
0 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants