Updated various notices, functionality issues, ajax issues, syntax, comments, and beefed up readme

[ryanurban]

So this actually takes the 3 pending pull requests from me and Kaiser and rolls them into one. I had to update how we were calling the ajax nonce and actions as submitting menu items was returning a 0. So with this, all the notices and issues are cleaned up and working for WordPress 3.5.1!

To recap this includes:

• https://github.com/franz-josef-kaiser/WordPress-Post-Type-Archive-Links/commit/16aed73ef52b8019eb6d2c7a3029816f888e6ba0
• https://github.com/ryanurban/WordPress-Post-Type-Archive-Links/commit/7f35216479aa0e93500cdfd0ef6e41d35c99d655
• https://github.com/ryanurban/WordPress-Post-Type-Archive-Links/commit/7deef73a58f88a0b7dc2c7e76c4ed4f254f139c7
• And finally fixing the broken ajax actions so menu items will get appended to our menu
• Made the function open brackets and array arguement commas more WordPressy

[stephenharris]

Wow. awesome guys! Thanks for your hard work on this. Its late Sunday where I am so I shall wait till tomorrow to give it the once over and merging it. Thanks again @ryanurban and @franz-joself-kaiser

[ryanurban]

You are very welcome, Stephen, our pleasure! Thanks for creating this plugin, as this is something I've wanted for awhile for clients. Happy to be apart of updating it, and really nice work Kaiser in the initial update, I learned several more awesome WordPress function and tags in looking through your code : )

[franz-josef-kaiser]

Sigh didn't get the notification for that issue...

I'll later run over it again and extend my pull requests with those fixes (the once I haven't got already in) and add @ryanurban as contributor. Btw: @ryanurban Do you have a login name for the wp.org repo for us?

[franz-josef-kaiser]

@ryanurban I hope you don't take that as an offense, but could you please make smaller commits the next time? The diffs on GitHub aren't as good as one would expect, so it basically looks like a complete rewrite. Best practice is to commit every single method or function that changed separately. When you do a push it will anyway reference the whole files as a single request and is easier scanable. Thanks :)

[ryanurban]

Hey @franz-josef-kaiser sorry about that, still learning how this aspect of GitHub works, and what's the easiest for everyone involved.

You'll see in one of my above commits (50b5c6d) that I updated the readme and included my WP.org name : )

Also, unless you have more code that you hadn't added and pull requested, the above commits include your outstanding pull request 16aed73

Some of my updates and fixes also had to do with resolving the warnings and notices I was seeing for the plugin with WP debug on.

Thanks and let me know if you have any questions, and again, sorry about that.

[franz-josef-kaiser]

@ryanurban For some reason I didn't get notified about this pull request. Therefore I incorporated anything myself. The one thing missing in my pull request (I always develop with "paranoia config" ) is the handling of the animated GIF when items get added to the menu.

[franz-josef-kaiser]

Well, "anything" is the things I saw, so there might be stuff that I missed due to the fact that I couldn't get a real diff from your pull request - not blaming anyone, we all started somewhere and every(!!) pull request helps. Maybe you want to do some inline commenting? You can simply click the line number and note where I missed something. I'll leave the readme out, so you can do that one :) Does that fit you? (And thanks for the help here:) )

[ryanurban]

I specifically removed that gif as I believe WordPress updated some of their markup, js, and css that was dealing with the gif, and as such, the gif was busted and always showing, or would always show once you added something to the menu.

That's the only thing I removed, otherwise I just updated all of the noted bits above. The other main thing that I updated, that would affect the overall plugin code, was overall syntax of things like brackets, nesting, array commas, etc per http://codex.wordpress.org/WordPress_Coding_Standards.

Otherwise I'm not sure I understand what's wrong here or what you're needing inline comments for. I mean feel free to look at my pull request (everything: https://github.com/ryanurban/WordPress-Post-Type-Archive-Links) and the plugin code without my pull requests.

I'm sure I'm just misunderstanding what you're needing from me here.

[ryanurban]

I'm happy to add the gif back in if you want to deal with the hide/show of it.

[franz-josef-kaiser]

You got me slightly wrong. With "inline comments" I meant here inside GitHub. You can click on one of your files for a pull request, click there on some line number and add a comment. This way you can guide me through your changes as the request itself shows everything as deleted and added as you changed the syntax...

[ryanurban]

Sure, I'll do that and do my best to catch and comment everything : ) I'll ignore the syntax bits as those should be fairly obvious and have no bearing on the real things that got fixed.

Sit tight.

[ryanurban]

The 4th screenshot is just to show all the WP nav menu classes being applied on the front-end, including the current menu item class.

[ryanurban]

Almost done with my commentary

[franz-josef-kaiser]

Very kool. Thanks :)

I'll open another issue to make the JS stand more in line with how core does it.

[ryanurban]

Okay so I went through the js and plugin code and did my best to comment out. I also, as you can see, edited the button markup and added the spinner markup back in.

I'm still trying to figure out though what I'm missing either markup or js wise on the hide/show of the spinner, as it isn't currently working at all.

Let me know your thoughts on all of the above.

[ryanurban]

Just about got this spinner working

[ryanurban]

Alright, done @franz-josef-kaiser! Spinner now works, we remove checked inputs after done appending, and updated the use of vars in the plugin code for use for the spinner and to make more sense as they were being used for metabox id vs metabox cpt list items.

Let me know if you have any additional questions about anything I've done!

[stephenharris]

Have I mucked this up? I had to manual resolve the merge, but now its not coming up with your names as contributors, and https://github.com/stephenharris/WordPress-Post-Type-Archive-Links/contributors doesn't list you either...?

In other news, @franz-josef-kaiser there was a small issue your commit 16aed73ef52b8019 (included in this too). You were calling add_action( 'admin_init', array( $this, 'add_meta_box' ) ); twice, and the one you added as part of optimisation turns out to run too late.

Everything else looked good, and testing didn't throw anything up. If alls well, I'll push this to WP repo Sunday/Monday.

[ryanurban]

Hmm not sure, all the files and code look right to me. Regarding those stats, maybe that only shows numbers for people with full privileges to the repo?

But yeah, everything looks good as far as I can tell. Let us know when that goes up, and maybe we can provide some help in the forums for it as well.

Thanks!

[franz-josef-kaiser]

There are still minor issues and one major issue left:

The minor, or better nano issues :)

• Styling: We got a pretty inconsistent bracket usage and bringing imho ! current_user_can('') with AND die() in the same line easily can be missed when reading. The only place where I use this is at the top of the file to prevent direct access as no one has to read it there.
• Styling: The new button has about 800k charts in a row. No one should be forced to scroll horizontally when reading code.
• The spinning circle sits on the wrong side of the button

Major issue:

• Currently we don't successfully disable / prevent hitting the button multiple times. This then adds an the same amount of items to the menu as you clicked the button. This shouldn't be that way.

[ryanurban]

Hey @franz-josef-kaiser to address your above points:

• Brackets have been updated to be consistent (f849e42). I had done this but think some of the updates got lost in @stephenharris's merge conflict
• I broke the button into 3 lines to be more readable, though I don't think this matters much, as people can typically select in their IDE of choice whether they want code to wrap or not (d5d0604), @stephenharris think whether or not you want to merge this request is totally up to you.
• I'm not seeing the spinner being on the wrong side of the button. Are you sure you're using the latest code? I see the spinner being on the left of the button which is correct as of 3.5.1 if you look at where the spinner is for the other buttons.
• I updated our jQuery to disable/rein-able our submit button, though once again, not sure this is necessary. If you test the other submit buttons as of at least 3.5.1 (maybe even 3.5), they are not getting disabled on click, and can be clicked multiple times (821d738). @stephenharris think whether or not you want to merge this request is totally up to you.

[franz-josef-kaiser]

Jo @ryanurban

I got a big problem with the way the WordPress coding standards wants their brackets. And as this is no part of core, we normally drop it and go with Egyptian bracket styling. This makes the difference between methods/functions much more clear.

About the spinner. I can see it on the right side while it appears on the left side for the default buttons. But nvm. This is so minor that no one will even notice it.

As I wrote in the other issue, the question is if we shouldn't just use the default markup and let WP core JS do the heavy lifting.

[ryanurban]

Hey guys, @franz-josef-kaiser, yeah I was and am seeing the spinner as working correctly and being to the left of the add to menu button, it should, as it has the same markup and classes as the other buttons and spinners do.

I definitely agree about letting WP core doing as much as possible, but I will say that the markup that we currently are using is the default WordPress metabox menu markup, it's probably more a matter of seeing how we can harness the WP core JS working the other menu metaboxes, as you're alluding to.

[ryanurban]

Though I definitely think that what we've done now is good for the next versino on .org : )

[stephenharris]

Cheers guys. I've finally made the time to release 1.1 on .org. Thanks for all your hard work :).

I'll leave discussion about the JS for the other thread - just to say here I'm not convinced how it can be done - details on the other thread.

[ryanurban]

Awesome, thanks Stephen!

[franz-josef-kaiser]

Perfect :)