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.

Sign up for GitHub

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

Jump to bottom

Remove inline javascript code from BuildTriggerStep #126

Merged
merged 4 commits into from
Oct 27, 2023
Merged

Remove inline javascript code from BuildTriggerStep #126

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4bf9d68
feat:Remove inline javascript code from BuoldTriggerStep
NeetigyaPod Oct 9, 2023
78a02d5
feat:Remove inline javascript code from BuoldTriggerStep
NeetigyaPod Oct 9, 2023
1fd04a2
feat:Remove inline javascript code from BuildTriggerStep
NeetigyaPod Oct 10, 2023
84643bf
Merge branch 'master' into feat/remove-inline-buildtrigger
NeetigyaPod Oct 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 4 additions & 17 deletions ...esources/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStep/config.jelly
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ THE SOFTWARE.
<j:jelly xmlns:j="jelly:core" xmlns:f="/lib/form" xmlns:st="jelly:stapler">
<j:set var="jobFieldId" value="${h.generateId()}"/>
Copy link
Member

Choose a reason for hiding this comment

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

In general I think we should be able to get rid of h.generateId() in these refactorings, by making the behavior function take the element argument into account and looking for siblings. Same for the hard-coded id params, which we did not even bother uniquifying using generateId since we expect this Jelly facet to be loaded only once in a page (Snippet generator).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh makes sense, I want to contribute to Jenkins in long term as well so I can work on it as a separate issue?
Also is there any slack for Jenkins or is Gitter the preferred platform?

Copy link
Member

Choose a reason for hiding this comment

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

I can work on it as a separate issue?

Sure, if you would like. It is not a priority, would just be a nice stylistic improvement.

is there any slack for Jenkins or is Gitter the preferred platform?

https://www.jenkins.io/chat/ though TBH I am not normally on those. Not sure why Slack is not used.

<f:entry field="job" title="Project to Build">
<f:textbox onblur="loadParams()" id="${jobFieldId}"/>
<span class="textbox-params-reference-holder" data-id="${jobFieldId}"/>
<f:textbox id="${jobFieldId}"/>
<st:adjunct includes="org.jenkinsci.plugins.workflow.support.steps.build.BuildTriggerStep.configLoad"/>
</f:entry>
<f:entry field="wait">
<f:checkbox default="true" title="Wait for completion"/>
Expand All @@ -42,21 +44,6 @@ THE SOFTWARE.
<f:number clazz="number"/>
</f:entry>
<f:entry field="parameters" title="Parameters">
<div id="params"/>
<script>
function loadParams() {
var div = document.getElementById('params');
fetch('${descriptor.descriptorUrl}/parameters?job=' + encodeURIComponent(document.getElementById('${jobFieldId}').value) + '&amp;context=${descriptor.contextEncoded}').then((rsp) => {
rsp.text().then((responseText) => {
if (rsp.ok) {
div.innerHTML = responseText;
Behaviour.applySubtree(div);
} else {
div.innerHTML = "<b>ERROR</b>: Failed to load parameter definitions: " + rsp.statusText;
}
});
});
}
</script>
<div id="params" data-jobfield = '${jobFieldId}' data-descriptor = '${descriptor.descriptorUrl}'/>
</f:entry>
</j:jelly>
18 changes: 18 additions & 0 deletions ...sources/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStep/configLoad.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
Behaviour.specify(".textbox-params-reference-holder", 'textbox-onblur-function', 0, function (e) {
var id = e.getAttribute('data-id');
var textbox = document.getElementById(id);

textbox.onblur = function(el) {
var div = document.getElementById('params');
fetch(document.querySelector('#params').dataset.descriptor + '/parameters?job=' + encodeURIComponent(document.getElementById(document.querySelector('#params').dataset.jobfield).value) + '&amp;context=${descriptor.contextEncoded}').then((rsp) => {
rsp.text().then((responseText) => {
if (rsp.ok) {
div.innerHTML = responseText;
Behaviour.applySubtree(div);
} else {
div.innerHTML = "<b>ERROR</b>: Failed to load parameter definitions: " + rsp.statusText;
}
});
});
}
});