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

Use new dialog design for adding credentials #500

Merged
merged 6 commits into from
Mar 1, 2024
+30 −67
Merged

Use new dialog design for adding credentials #500

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
54e9df2
Use new dialog design
zbynek Jan 6, 2024
8fa0f51
Update DOM selector for dialog test
zbynek Jan 6, 2024
a4b212c
Merge branch 'master' into dialog-update
zbynek Feb 27, 2024
4fa8dbf
Remove unused adjunct
zbynek Feb 28, 2024
3a9a025
Update form markup, make it work with HtmlUnit
zbynek Feb 29, 2024
a96c5b5
Update jenkins.version and bom
timja Mar 1, 2024
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
2 changes: 1 addition & 1 deletion pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@
<properties>
<changelist>999999-SNAPSHOT</changelist>
<gitHubRepo>jenkinsci/${project.artifactId}-plugin</gitHubRepo>
<jenkins.version>2.387.3</jenkins.version>
<jenkins.version>2.426.1</jenkins.version>
timja marked this conversation as resolved.
Show resolved Hide resolved
</properties>

<repositories>
Expand Down
37 changes: 14 additions & 23 deletions ...loudbees/plugins/credentials/CredentialsSelectHelper/WrappedCredentialsStore/dialog.jelly
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,29 +25,20 @@
<?jelly escape-by-default='true'?>
<j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:f="/lib/form" xmlns:l="/lib/layout">
<l:ajax>
<div class="hd">
<h2 style="margin-top: 1rem" tooltip="${it.description}">${it.description}: ${it.displayName}</h2>
</div>
<div class="bd">
<h3>${%Add Credentials}</h3>
<form action="${it.url}/addCredentials" method="POST" id="credentials-dialog-form">
<table width="100%">
<f:entry title="${%Domain}">
<select class="setting-input" name="_.domain">
<j:forEach var="domain" items="${it.wrappers.entrySet()}">
<f:option value="${domain.key}" selected="${domain.key=='_'}">${domain.value.displayName}</f:option>
</j:forEach>
</select>
</f:entry>
<f:block>
<j:set var="descriptors" value="${it.credentialsDescriptors}"/>
<st:include page="credential"/>
</f:block>
<f:block>
<input id="credentials-add-submit" class="yui-button primary" value="${%Add}"/>
<input id="credentials-add-abort" class="yui-button" value="${%Cancel}"/>
</f:block>
</table>
<div>
<form action="${it.url}/addCredentials" method="POST" id="credentials-dialog-form" data-title="${it.description}: ${it.displayName}" data-add="${%Add}">
Copy link

Choose a reason for hiding this comment

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

I think it would be better to have the Add Credentials in the title as well
e.g. Add Credentials (Provider: Jenkins)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea, but I'd suggest to do that in a follow-up PR and keep this one just about the design.

<h3>${%Add Credentials}</h3>
<f:entry title="${%Domain}">
<select class="setting-input" name="_.domain">
<j:forEach var="domain" items="${it.wrappers.entrySet()}">
<f:option value="${domain.key}" selected="${domain.key=='_'}">${domain.value.displayName}</f:option>
</j:forEach>
</select>
</f:entry>
<f:block>
<j:set var="descriptors" value="${it.credentialsDescriptors}"/>
<st:include page="credential"/>
</f:block>
</form>
</div>
</l:ajax>
Expand Down
7 changes: 0 additions & 7 deletions src/main/resources/lib/credentials/dialog.js
View file
Open in desktop

This file was deleted.

1 change: 0 additions & 1 deletion src/main/resources/lib/credentials/select.jelly
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@
<j:set var="selectHelper" value="${app.getDescriptorByName('com.cloudbees.plugins.credentials.CredentialsSelectHelper')}"/>
<j:set var="includeUser" value="${attrs.includeUser ? true : false}"/>
<j:set var="context" value="${selectHelper.resolveContext(attrs.context ?: it)}"/>
<st:adjunct includes="lib.credentials.dialog"/>
<div class="credentials-select-control">
<j:if test="${attrs.expressionAllowed}">
<label field-disabled="true">
Expand Down
1 change: 0 additions & 1 deletion src/main/resources/lib/credentials/select/select.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
select.credentials-select { width:auto; vertical-align: top; }
div.credentials-select-content-inactive { display:none; }
div#credentialsDialog {overflow-y: scroll;}
body.masked {overflow-y: hidden;}
div.include-user-credentials { padding: 0.2em; }
/* adapted from Jenkins' style.css */
Expand Down
41 changes: 11 additions & 30 deletions src/main/resources/lib/credentials/select/select.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,22 +28,6 @@ window.credentials.init = function () {
document.body.appendChild(div);
div.innerHTML = "<div id='credentialsDialog'><div class='bd'></div></div>";
window.credentials.body = document.getElementById('credentialsDialog');
window.credentials.dialog = new YAHOO.widget.Panel(window.credentials.body, {
fixedcenter: true,
close: true,
draggable: true,
zindex: 1000,
modal: true,
visible: false,
keylisteners: [
new YAHOO.util.KeyListener(document, {keys:27}, {
fn:(function() {window.credentials.dialog.hide();}),
scope:document,
correctScope:false
})
]
});
window.credentials.dialog.render();
}
};
window.credentials.add = function (e) {
Expand All @@ -54,15 +38,15 @@ window.credentials.add = function (e) {
}).then(rsp => {
if (rsp.ok) {
rsp.text().then((responseText) => {
// do not apply behaviour on parsed HTML, dialog.form does that later
// otherwise we have crumb and json fields twice
window.credentials.body.innerHTML = responseText;
Behaviour.applySubtree(window.credentials.body, false);
window.credentials.form = document.getElementById('credentials-dialog-form');
// window.credentials.form.action = e;
var r = YAHOO.util.Dom.getClientRegion();
window.credentials.dialog.cfg.setProperty("width", r.width * 3 / 4 + "px");
window.credentials.dialog.cfg.setProperty("height", r.height * 3 / 4 + "px");
window.credentials.dialog.center();
window.credentials.dialog.show();
const data = window.credentials.form.dataset;
const options = {'title': data['title'], 'okText': data['add'], 'submitButton':false, 'minWidth': '75vw'};
dialog.form(window.credentials.form, options)
.then(window.credentials.addSubmit);
window.credentials.form.querySelector('select').focus();
});
}
});
Expand Down Expand Up @@ -109,9 +93,6 @@ window.credentials.refreshAll = function () {
if (window.console != null) {
console.warn("Unable to find nearby " + name);
}
if (window.YUI != null) {
YUI.log("Unable to find a nearby control of the name " + name, "warn")
}
return;
}
deps.push({name: Path.tail(name), control: c});
Expand All @@ -121,9 +102,12 @@ window.credentials.refreshAll = function () {
});
};
window.credentials.addSubmit = function (_) {
const form = document.getElementById('credentials-dialog-form');
const form = window.credentials.form;
// temporarily attach to DOM (avoid https://github.com/HtmlUnit/htmlunit/issues/740)
document.body.appendChild(form);
buildFormTree(form);
ajaxFormSubmit(form);
form.remove();

function ajaxFormSubmit(form) {
fetch(form.action, {
Expand All @@ -140,9 +124,6 @@ window.credentials.addSubmit = function (_) {
// notificationBar.show(...) with logging ID could be handy here?
console.error("Could not add credentials:", e);
})
.finally(() => {
window.credentials.dialog.hide();
});
}
};

Expand Down
4 changes: 2 additions & 2 deletions src/test/java/com/cloudbees/plugins/credentials/CredentialsSelectHelperTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public void doAddCredentialsFromPopupWorksAsExpected() throws Exception {
HtmlListItem li = htmlPage.querySelector(".credentials-add-menu-items li");
li.click();
wc.waitForBackgroundJavaScript(4000);
HtmlForm form = htmlPage.querySelector("#credentialsDialog form");
HtmlForm form = htmlPage.querySelector("#credentials-dialog-form");

HtmlInput username = form.querySelector("input[name='_.username']");
username.setValue("bob");
Expand All @@ -40,7 +40,7 @@ public void doAddCredentialsFromPopupWorksAsExpected() throws Exception {
HtmlInput id = form.querySelector("input[name='_.id']");
id.setValue("test");

HtmlSpan formSubmitButton = form.querySelector("#credentials-add-submit");
HtmlButton formSubmitButton = htmlPage.querySelector(".jenkins-button[data-id='ok']");
formSubmitButton.fireEvent("click");
wc.waitForBackgroundJavaScript(5000);

Expand Down