search

kevinburke / doony

UI Improvements for Jenkins
http://doony.org
Other
970 stars 116 forks source link

Correct "Build Now" and "Build with Parameters" buttons. #53

Closed wilmoore closed 9 years ago

wilmoore commented 10 years ago

Summary

Attempts to fix #5 by:

Job With Parameters...Next Page

Normal Job

Build Now

Normal Job after Click

after click

kevinburke commented 10 years ago

It looks like the button no longer works for regular builds; the button text is now "undefined" for simple jobs. Looking into it.

On 1/29/14, 10:33, Wil Moore III wrote:

Attempts to fix #5 https://github.com/kevinburke/doony/issues/5 by:

  • Adding a |getButtonInfo| function which returns normalized button key/values (i.e. innerHTML, href, hasParams?).
  • Following same approach as links in that there is a simple |onclick| vs. |jQuery.click|.
  • Delegating to the Jenkins intrinsic |Ajax.Request| for dispatching request.
  • Using |hoverNotification|.
    You can merge this Pull Request by running

git pull https://github.com/wilmoore/doony patch-1

Or view, comment on, or merge it at:

https://github.com/kevinburke/doony/pull/53

    Commit Summary

— Reply to this email directly or view it on GitHub https://github.com/kevinburke/doony/pull/53.

kevinburke commented 10 years ago

Also I'm not sure what hoverNotification is or is supposed to be? a google search didn't turn up much. is it a Jenkins function?

wilmoore commented 10 years ago

hoverNotification is a Jenkins provided function. If you are in chrome, hover over the "Build Now" link within a job and go to "Inspect Element". You should find a global (i.e. attached to window) onclick handler. If you type into the console, window.handlerName (where handlerName is whatever name you found), you'll see the contents of the handler which includes hoverNotification.

wilmoore commented 10 years ago

It looks like the button no longer works for regular builds; the button text is now "undefined" for simple jobs. Looking into it.

That's interesting. It worked fine for me -- I wonder if it matters that I am running and tested this on version 1.549?

wilmoore commented 10 years ago

the button text is now "undefined" for simple jobs. Looking into it.

Ah, could it be that you are not logged into your Jenkins instance ATM? Not sure why I decided to log-out, but I did, and I noticed the undefined button text. It seems that the "Build Now" link does not show up when you are logged out, so we never find a node with the metatdata we are looking for. This should actually trigger us to not show a build button to be consistent with the link.

Going to get some sleep, but will update the PR when I have a moment.

wilmoore commented 10 years ago

So, my latest commit corrects the undefined text issue when user is not logged-in. If the user is not logged-in, the button (just like the link) does not appear.

wilmoore commented 10 years ago

bump :)

kevinburke commented 10 years ago

Sorry!!! Will get to this soon :0

Kevin Burke | Twilio phone: 925.271.7005 | kev.inburke.com

On Sat, Apr 19, 2014 at 11:08 AM, Wil Moore III notifications@github.comwrote:

bump :)

— Reply to this email directly or view it on GitHubhttps://github.com/kevinburke/doony/pull/53#issuecomment-40861548 .

wilmoore commented 10 years ago

Sounds good :+1:

supermarin commented 10 years ago

@kevinburke also to keep in mind - Build now button shouldn't be shown if the user isn't logged in

aaronmaxlevy commented 10 years ago

The RegEx used by isJobPage could be improved. This currently returns false for a job page that is opened from a List View.

AlmogBaku commented 9 years ago

+1

wilmoore commented 9 years ago

FYI, I've since moved on from Jenkins so hopefully someone more enthusiastic about continuing with Jenkins will pick this up going forward.

kevinburke commented 9 years ago

I'm sorry I haven't been great at this - I think it's a great idea, I just haven't had the time to verify the correctness of this.

Should I just merge it? I probably won't know how to fix it if it's broken either.

Kevin Burke phone: 925.271.7005 | kev.inburke.com

On Tue, Aug 26, 2014 at 9:28 AM, Wil Moore III notifications@github.com wrote:

Closed #53 https://github.com/kevinburke/doony/pull/53.

— Reply to this email directly or view it on GitHub https://github.com/kevinburke/doony/pull/53#event-157404960.

wilmoore commented 9 years ago

Should I just merge it? I probably won't know how to fix it if it's broken

Probably not. Seems like there may be edge cases around the regex guard; however, if anyone wants to pick it up from there, seems like it's close.