[JENKINS-35333] New job popup #2388

samatdav · 2016-06-01T20:37:38Z

This Pull Request implements the ability to create a new job in a popup dialog window directly from home page.
The change uses remodal library which uses css in an added file and required js from npm module.
The interface of the popup is taken from existing newJob.jelly structure and placed into a popup.
You can see the demonstration of the change in the GIF below.

lanwen · 2016-06-02T10:02:23Z

war/src/main/webapp/css/style.css

+
+
+
+html.remodal-is-locked{overflow:hidden;-ms-touch-action:none;touch-action:none}.remodal,[data-remodal-id]{display:none}.remodal-overlay{position:fixed;z-index:9999;top:-5000px;right:-5000px;bottom:-5000px;left:-5000px;display:none;background:rgba(43,46,56,.9)}.remodal-wrapper{position:fixed;z-index:10000;top:0;right:0;bottom:0;left:0;display:none;overflow:auto;text-align:center;-webkit-overflow-scrolling:touch;padding:10px 10px 0}.remodal-wrapper:after{display:inline-block;height:100%;margin-left:-.05em;content:""}.remodal-overlay,.remodal-wrapper{-webkit-backface-visibility:hidden;backface-visibility:hidden}.remodal{position:relative;outline:0;-webkit-text-size-adjust:100%;-moz-text-size-adjust:100%;-ms-text-size-adjust:100%;text-size-adjust:100%;-webkit-box-sizing:border-box;box-sizing:border-box;margin-bottom:10px;padding:35px;-webkit-transform:translate3d(0,0,0);transform:translate3d(0,0,0);color:#2b2e38;background:#fff}.remodal-cancel,.remodal-close,.remodal-confirm{overflow:visible;margin:0;cursor:pointer;text-decoration:none;outline:0;border:0}.remodal-is-initialized{display:inline-block}.remodal-close,.remodal-close:before{position:absolute;top:0;left:0;display:block;width:35px}.remodal-bg.remodal-is-opened,.remodal-bg.remodal-is-opening{-webkit-filter:blur(3px);filter:blur(3px)}.remodal-overlay.remodal-is-closing,.remodal-overlay.remodal-is-opening{-webkit-animation-duration:.3s;animation-duration:.3s;-webkit-animation-fill-mode:forwards;animation-fill-mode:forwards}.remodal-overlay.remodal-is-opening{-webkit-animation-name:remodal-overlay-opening-keyframes;animation-name:remodal-overlay-opening-keyframes}.remodal-overlay.remodal-is-closing{-webkit-animation-name:remodal-overlay-closing-keyframes;animation-name:remodal-overlay-closing-keyframes}.remodal.remodal-is-closing,.remodal.remodal-is-opening{-webkit-animation-duration:.3s;animation-duration:.3s;-webkit-animation-fill-mode:forwards;animation-fill-mode:forwards}.remodal.remodal-is-opening{-webkit-animation-name:remodal-opening-keyframes;animation-name:remodal-opening-keyframes}.remodal.remodal-is-closing{-webkit-animation-name:remodal-closing-keyframes;animation-name:remodal-closing-keyframes}.remodal,.remodal-wrapper:after{vertical-align:middle}.remodal-close{height:35px;padding:0;-webkit-transition:color .2s;transition:color .2s;color:#95979c;background:0 0}.remodal-close:focus,.remodal-close:hover{color:#2b2e38}.remodal-close:before{font-family:Arial,"Helvetica CY","Nimbus Sans L",sans-serif!important;font-size:25px;line-height:35px;content:"\00d7";text-align:center}.remodal-cancel,.remodal-confirm{font:inherit;display:inline-block;min-width:110px;padding:12px 0;-webkit-transition:background .2s;transition:background .2s;text-align:center;vertical-align:middle}.remodal-confirm{color:#fff;background:#81c784}.remodal-confirm:focus,.remodal-confirm:hover{background:#66bb6a}.remodal-cancel{color:#fff;background:#e57373}.remodal-cancel:focus,.remodal-cancel:hover{background:#ef5350}.remodal-cancel::-moz-focus-inner,.remodal-close::-moz-focus-inner,.remodal-confirm::-moz-focus-inner{padding:0;border:0}@-webkit-keyframes remodal-opening-keyframes{from{-webkit-transform:scale(1.05);transform:scale(1.05);opacity:0}to{-webkit-transform:none;transform:none;opacity:1}}@keyframes remodal-opening-keyframes{from{-webkit-transform:scale(1.05);transform:scale(1.05);opacity:0}to{-webkit-transform:none;transform:none;opacity:1}}@-webkit-keyframes remodal-closing-keyframes{from{-webkit-transform:scale(1);transform:scale(1);opacity:1}to{-webkit-transform:scale(.95);transform:scale(.95);opacity:0}}@keyframes remodal-closing-keyframes{from{-webkit-transform:scale(1);transform:scale(1);opacity:1}to{-webkit-transform:scale(.95);transform:scale(.95);opacity:0}}@-webkit-keyframes remodal-overlay-opening-keyframes{from{opacity:0}to{opacity:1}}@keyframes remodal-overlay-opening-keyframes{from{opacity:0}to{opacity:1}}@-webkit-keyframes remodal-overlay-closing-keyframes{from{opacity:1}to{opacity:0}}@keyframes remodal-overlay-closing-keyframes{from{opacity:1}to{opacity:0}}


does @import directive works?

You mean importing like that? <style type="text/css" media="screen">@import "${cssURL}/popup.css";</style>
And then placing popup.css in ${cssURL} folder?
It seems that ${cssURL} is not defined when I try to include into sidepanel.jelly

is this used somewhere else?

I found it here https://github.com/jenkinsci/iphoneview-plugin/blob/master/src/main/resources/hudson/plugins/iphoneview/tags/layout.jelly

So is it a right way way to import the library? Should CSS and JS be imported the same way? So is there a need to use js-modules? Tom talked about creation of detached version of remodal library - to not use jQuery in a global scope. Should I do it?

You should save library css in separate file - thats why you need import statement.

Yes, I tried to put css to another file, but could not import it (got file "404 not found").
Can I use <style> @import url('containingFolder/popup.css'); </style> ?
If so, what containingFolder would be?

It seems that now I have only one rule now. Should I put it in add-item.css ? Seems like a good place for it.
However, I still don't quite understand how to import remodal library rules. Are there any existing examples of importing css like that?

I'd think you'll need to bundle the remodal CSS inside in Jenkins and use an adjunct or <l:css src="..." />

Maybe as example:
https://github.com/jenkinsci/github-plugin/blob/master/src/main/resources/org/jenkinsci/plugins/github/admin/GitHubHookRegisterProblemMonitor/index.groovy#L13

oleg-nenashev · 2016-06-02T10:32:08Z

Please create a JIRA issue with the new feature description

lanwen · 2016-06-02T10:46:48Z

core/src/main/resources/hudson/model/View/sidepanel.jelly

+      <l:task href="#modal" icon="icon-new-package icon-md" title="New Item Popup"/>
+      <!-- <st:include page="popup.jelly" optional="true" /> -->
+
+      <st:include optional="true">


what include (optional), but without any other args do?

Apparently, nothing. I though I needed to include html between these tags. However, it seems that I dont. I will remove them.

samatdav · 2016-06-03T14:00:45Z

@oleg-nenashev I created a JIRA issue. https://issues.jenkins-ci.org/browse/JENKINS-35333
Please comment if there is anything wrong.

samatdav · 2016-06-05T07:21:25Z

core/src/main/resources/hudson/model/View/sidepanel.jelly

+              <form method="post" action="createItem" name="createItem" id="createItem">
+                <div class="header">
+                  <div class="add-item-name">
+                    <label for="name">${%ItemName.label}</label>


So where can I look to fix replacements of ${%ItemName.label} by a corresponding string?

What is ${%ItemName.label} supposed to be? The % looks dodgy. Rings JSP bells.

ItemName does not look like an instance id of any kind. It looks more like a raw type name. What are you expecting to happen here? Where are you expecting the values to come from?

${%ItemName.label} is localization marker. Try to search .properties files with such key

Yes, that was it. I fixed the problem by adding newJob properties to sidepanel.properties

recena · 2016-06-05T09:23:52Z

This PR should not be merged until this #2372 is finished.

recena · 2016-06-05T09:26:53Z

@samatdav

Include the JIRA ID in the PR title
If you are working on JENKINS-35333, assign it yourself and update the state of the issue.

recena · 2016-06-05T09:28:11Z

war/src/main/js/add-item.js

@@ -107,7 +107,6 @@ $.when(getItems()).done(function(data) {
          btn.removeClass('disabled');
          btn.prop('disabled', false);
        }
-        btn.focus();


If the GET request sent after each character and all three checks pass (js allows to create a job) then the enableSubmit function is called and OK button is focused after each character typed.
Kirill suggested not to remove this focus and create different functions for blur and keyup events. So this is not finished also.

I included JIRA ID in PR title and assigned myselft in JIRA

👎 The current behavior was deeply discussed with @gusreiber. At any rate, it seems there are a lot of code in progress. I'll wait until something more stable.

Sure, I agree. Since most of the issues with new job name check were solved recently, check of name validity after each typed character is my only suggestion for the improvement left.

recena · 2016-06-05T09:32:46Z

@samatdav FYI https://wiki.jenkins-ci.org/display/JENKINS/Instructions+for+Committers

samatdav · 2016-06-07T23:48:03Z

core/src/main/resources/hudson/model/View/sidepanel.jelly

+      <l:task href="#modal" icon="icon-new-package icon-md" title="Remodal Popup"/>
+      <!-- <st:include page="popup.jelly" optional="true" /> -->
+
+      <div class="remodal new-job-popup" data-remodal-id="modal" id="add-item-panel">


@lanwen is this better now? I successfully removed bootstrap components (container, row, etc.) and made the popup work without it. You can also see that I changed styles. And now the only rule I need is for .new-job-popup

what for you need remodal class?

I think this is how the library references element. Without this class the popup does not even open. I believe this is like bootstrap modal which also contains .modal in it.

can you point lib to search .new-job-popup?

I probably don't know how to do it yet. Do you have ideas how?
I always used library styles by referring to their class and though this is how they should be used.

What documentation of lib says about retargeting? Did you tried to read the library code?

lanwen · 2016-06-13T18:39:18Z

Can you detach commits about new look and just make this pr as popup? Just to be prepared to merge. (it should pass tests at minimal state). Then new look should be in separate pr on top of this pr, as of it can take some long time to be reviewed by core-maintainers.

lanwen · 2016-06-14T19:29:56Z

core/src/main/resources/hudson/model/View/sidepanel.jelly

@@ -43,6 +47,9 @@ THE SOFTWARE.
        <l:task href="${rootURL}/${it.viewUrl}newJob" icon="icon-new-package icon-md" it="${app}" permission="${it.itemCreatePermission}" title="${%NewJob(it.newPronoun)}"/>
      </st:include>

+      <l:task href="#modal" icon="icon-new-package icon-md" title="Remodal Popup"/>


just replace existing - no need to duplicate

samatdav · 2016-06-16T17:40:05Z

I created two additional pull requests (at samatdav/jenkins) to have different PRs for different features.

This PR is for popup modal dialog
New Job Name Validation PR is for improvements in javascript client side name validation
New Job UI Change PR is for new proposed style changes

samatdav · 2016-06-19T19:38:22Z

@lanwen It seems that many tests fail because of Remodal JavaScript library inclusion. Where and how I should include it? The popup works locally by using two lines:

//jQuery min js
//Remodal min js

lanwen · 2016-06-20T11:17:59Z

It would be nice to attach remodal.js to commit if you want to bundle it to jenkins. Now don't see it in any place

lanwen · 2016-06-20T11:20:39Z

Also you should put your war/src/main/webapp/jsbundles/remodal.css to smth like war/src/main/js/widgets/add/remodal.css and be sure it copied to jsbundle location.
As of jsbundle should not contain code - it should be completely autofilled place.

lanwen · 2016-06-20T11:22:58Z

Please provide current view in description (you have gif recorded)

samatdav · 2016-06-29T13:51:01Z

@lanwen please take a look at the new changes. With the help of Tom, the remodal js and css files are used properly. Now, all the tests are passed without failures.
However, just as in name validation the build still fails. Why is that?
What would you suggest to do next?

lanwen · 2016-06-29T19:57:09Z

war/hs_err_pid3655.log

@@ -0,0 +1,692 @@
+#


why you've commited this file?

.gitignore needs to exclude .log files or hs*log at least

That was an accident. I did not change gitignore settings so I don't know why that was committed. Now I removed it. I think may be I will make a new pull request later with one commit.

michaelneale · 2016-07-27T08:23:13Z

lint error 07:38:37 [INFO] src/main/js/add-item.js: line 3, col 5, '$remodal' is defined but never used.

michaelneale · 2016-07-27T08:57:03Z

ping @samatdav

michaelneale · 2016-07-27T08:58:54Z

as mentioned by @daniel-beck this needs to honour current behavior of adding things to the currently selected view

lanwen · 2016-08-11T16:32:52Z

@samatdav did you fix problem related to default view when its not "All"?

samatdav · 2016-08-12T12:37:13Z

@oleg-nenashev @lanwen @recena @michaelneale @tfennelly @recena Could you please review the pull request?
@recena do you have objections against remodal library? If so what do you suggest to do?

lanwen · 2016-08-12T20:06:03Z

LGTM

michaelneale · 2016-08-17T09:25:39Z

Current state of this is that remodal should not be used (cc @recena - is this correct? as per your comment on #2501).

installWizard uses bootstrap and no modal library, however.

oleg-nenashev · 2016-08-27T20:27:34Z

IIRC the decision was not to merge it in the current state

daniel-beck · 2016-12-19T18:45:08Z

This PR appears to have been abandoned.

lanwen reviewed Jun 2, 2016
View reviewed changes

oleg-nenashev added the work-in-progress The PR is under active development, not ready to the final review label Jun 2, 2016

oleg-nenashev changed the title ~~[WiP - for GSOC mentors] New job popup~~ [WiP] New job popup Jun 2, 2016

lanwen reviewed Jun 2, 2016
View reviewed changes

samatdav reviewed Jun 5, 2016
View reviewed changes

recena reviewed Jun 5, 2016
View reviewed changes

samatdav changed the title ~~[WiP] New job popup~~ [JENKINS-35333] [WiP] New job popup Jun 5, 2016

samatdav reviewed Jun 7, 2016
View reviewed changes

lanwen reviewed Jun 14, 2016
View reviewed changes

lanwen reviewed Jun 29, 2016
View reviewed changes

samatdav force-pushed the master branch 11 times, most recently from 77c6509 to d43de11 Compare July 26, 2016 11:31

Added popup for new job creation

2c72ea0

samatdav force-pushed the master branch from d43de11 to 2c72ea0 Compare July 26, 2016 11:32

remover variable

11f52c3

kzantow mentioned this pull request Aug 6, 2016

[FIXED JENKINS-37232] - starting with 2.7 the ATH began to fail during create item jenkinsci/acceptance-test-harness#168

Merged

cleanup

ed63cd4

action view handling

958bc32

daniel-beck added needs-more-reviews Complex change, which would benefit from more eyes and removed work-in-progress The PR is under active development, not ready to the final review labels Aug 15, 2016

oleg-nenashev added the proposed-for-close There is no consensus about feasibility of this PR, and maintainers do not see a way forward for it label Aug 27, 2016

daniel-beck closed this Dec 19, 2016




		html.remodal-is-locked{overflow:hidden;-ms-touch-action:none;touch-action:none}.remodal,[data-remodal-id]{display:none}.remodal-overlay{position:fixed;z-index:9999;top:-5000px;right:-5000px;bottom:-5000px;left:-5000px;display:none;background:rgba(43,46,56,.9)}.remodal-wrapper{position:fixed;z-index:10000;top:0;right:0;bottom:0;left:0;display:none;overflow:auto;text-align:center;-webkit-overflow-scrolling:touch;padding:10px 10px 0}.remodal-wrapper:after{display:inline-block;height:100%;margin-left:-.05em;content:""}.remodal-overlay,.remodal-wrapper{-webkit-backface-visibility:hidden;backface-visibility:hidden}.remodal{position:relative;outline:0;-webkit-text-size-adjust:100%;-moz-text-size-adjust:100%;-ms-text-size-adjust:100%;text-size-adjust:100%;-webkit-box-sizing:border-box;box-sizing:border-box;margin-bottom:10px;padding:35px;-webkit-transform:translate3d(0,0,0);transform:translate3d(0,0,0);color:#2b2e38;background:#fff}.remodal-cancel,.remodal-close,.remodal-confirm{overflow:visible;margin:0;cursor:pointer;text-decoration:none;outline:0;border:0}.remodal-is-initialized{display:inline-block}.remodal-close,.remodal-close:before{position:absolute;top:0;left:0;display:block;width:35px}.remodal-bg.remodal-is-opened,.remodal-bg.remodal-is-opening{-webkit-filter:blur(3px);filter:blur(3px)}.remodal-overlay.remodal-is-closing,.remodal-overlay.remodal-is-opening{-webkit-animation-duration:.3s;animation-duration:.3s;-webkit-animation-fill-mode:forwards;animation-fill-mode:forwards}.remodal-overlay.remodal-is-opening{-webkit-animation-name:remodal-overlay-opening-keyframes;animation-name:remodal-overlay-opening-keyframes}.remodal-overlay.remodal-is-closing{-webkit-animation-name:remodal-overlay-closing-keyframes;animation-name:remodal-overlay-closing-keyframes}.remodal.remodal-is-closing,.remodal.remodal-is-opening{-webkit-animation-duration:.3s;animation-duration:.3s;-webkit-animation-fill-mode:forwards;animation-fill-mode:forwards}.remodal.remodal-is-opening{-webkit-animation-name:remodal-opening-keyframes;animation-name:remodal-opening-keyframes}.remodal.remodal-is-closing{-webkit-animation-name:remodal-closing-keyframes;animation-name:remodal-closing-keyframes}.remodal,.remodal-wrapper:after{vertical-align:middle}.remodal-close{height:35px;padding:0;-webkit-transition:color .2s;transition:color .2s;color:#95979c;background:0 0}.remodal-close:focus,.remodal-close:hover{color:#2b2e38}.remodal-close:before{font-family:Arial,"Helvetica CY","Nimbus Sans L",sans-serif!important;font-size:25px;line-height:35px;content:"\00d7";text-align:center}.remodal-cancel,.remodal-confirm{font:inherit;display:inline-block;min-width:110px;padding:12px 0;-webkit-transition:background .2s;transition:background .2s;text-align:center;vertical-align:middle}.remodal-confirm{color:#fff;background:#81c784}.remodal-confirm:focus,.remodal-confirm:hover{background:#66bb6a}.remodal-cancel{color:#fff;background:#e57373}.remodal-cancel:focus,.remodal-cancel:hover{background:#ef5350}.remodal-cancel::-moz-focus-inner,.remodal-close::-moz-focus-inner,.remodal-confirm::-moz-focus-inner{padding:0;border:0}@-webkit-keyframes remodal-opening-keyframes{from{-webkit-transform:scale(1.05);transform:scale(1.05);opacity:0}to{-webkit-transform:none;transform:none;opacity:1}}@keyframes remodal-opening-keyframes{from{-webkit-transform:scale(1.05);transform:scale(1.05);opacity:0}to{-webkit-transform:none;transform:none;opacity:1}}@-webkit-keyframes remodal-closing-keyframes{from{-webkit-transform:scale(1);transform:scale(1);opacity:1}to{-webkit-transform:scale(.95);transform:scale(.95);opacity:0}}@keyframes remodal-closing-keyframes{from{-webkit-transform:scale(1);transform:scale(1);opacity:1}to{-webkit-transform:scale(.95);transform:scale(.95);opacity:0}}@-webkit-keyframes remodal-overlay-opening-keyframes{from{opacity:0}to{opacity:1}}@keyframes remodal-overlay-opening-keyframes{from{opacity:0}to{opacity:1}}@-webkit-keyframes remodal-overlay-closing-keyframes{from{opacity:1}to{opacity:0}}@keyframes remodal-overlay-closing-keyframes{from{opacity:1}to{opacity:0}}

[JENKINS-35333] New job popup #2388

[JENKINS-35333] New job popup #2388

Conversation

samatdav commented Jun 1, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oleg-nenashev commented Jun 2, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samatdav commented Jun 3, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

recena commented Jun 5, 2016

recena commented Jun 5, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samatdav Jun 5, 2016 • edited

Choose a reason for hiding this comment

recena commented Jun 5, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lanwen commented Jun 13, 2016

Choose a reason for hiding this comment

samatdav commented Jun 16, 2016

samatdav commented Jun 19, 2016

lanwen commented Jun 20, 2016

lanwen commented Jun 20, 2016

lanwen commented Jun 20, 2016

samatdav commented Jun 29, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samatdav Jun 30, 2016 • edited

Choose a reason for hiding this comment

michaelneale commented Jul 27, 2016

michaelneale commented Jul 27, 2016

michaelneale commented Jul 27, 2016

lanwen commented Aug 11, 2016

samatdav commented Aug 12, 2016

lanwen commented Aug 12, 2016

michaelneale commented Aug 17, 2016 • edited

oleg-nenashev commented Aug 27, 2016

daniel-beck commented Dec 19, 2016

samatdav commented Jun 1, 2016 •

edited

samatdav Jun 5, 2016 •

edited

samatdav Jun 30, 2016 •

edited

michaelneale commented Aug 17, 2016 •

edited