CUNY Academic Commons

Cool. Thanks, Boone.

Dom, I've added you as a watcher to this, in case you feel like taking it on. If so, please write as a patch to BuddyPress trunk - http://buddypress.svn.wordpress.org/trunk/ I'm leaving it assigned to myself for the moment, but don't hesitate to snatch it away if it sounds interesting to you.

I'm plugging away on clearing latest updates from member-header. But this does sound interesting. I'm planning on knocking out as many of my current issues as I can over spring break (this coming week), I'll look into this after seeing how many of my current issues I can close out. This does sound like a fun challenge. :-)

I'll jump on this as soon as I can.

Boone,

I started researching this issue but I really need to have another call with you like the one we had about WordPress/BuddyPress RESTful URLs. I don't fully understand what needs to be changed, my BuddyPress admin skills are still very under-developed. Would you please let me know when you have 20-30 min free for a call? I'm pretty open, my schedule is being split between Commons and personal coding projects right now.

I read your posts on wp plugin development using git-svn and have decided that I will use that workflow if/when I start releasing public plugins to the wp plugin svn repo. For this issue I've tried to go a little simpler since I will only be editing like 2 files to get the functionality implemented. So I'm using plain vanilla svn for this feature. Here's what I've done so far:

1.) checked out wp trunk into a wp_svn working dir
2.) checked out bp trunk into wp_svn/wp-content/plugins dir
3.) ran the bp installer

Once all that was setup and running I started reading through the bp-xprofile files we discussed. I see how the js and php are interacting to enable the sortable feature and handle the callbacks to make sure the db reflects the changes.

I've edited the php output for the profile options to include the relevant sortable classes, copied the sortable js code from profile fields and altered it to make the options sortable. So far the interface seems to be working. I even added some js to rename the "Option 1:", "Option 2:", etc labels after the drag and drop happens. I've also been trying hard to change as little of the bp core code as possible. I figure a patch that doesn't "change everything" has a better chance of being reviewed and accepted?

There's one part of the interface I'm not sure how to handle though. Each option has a [x] link that deletes that option through ajax. Can you think of a way (off the top of your head) to dynamically update those links after a drag and drop? What I'm worried about is the urls for those links specify specific option numbers, I want to make sure that after a reordering, those links aren't pointing to the wrong option numbers in the db.

So all I really need to do now is add the php to handle the ajax callback that will reorder the options in the db. Obviously right now what happens is the elements are reordered in the ui and then when you refresh the page they all go back to the order stored in the db. I haven't started that backend db code yet, so I'll be back with a mound of questions about that soon. :-)

Since this is being done outside of the cac repo, is there some way you want me to send you what I'm working on? Should I practice making a patch file and email it to you (or attach it to an update here, or even on that trac ticket you referenced above) for you to check my progress?

I figure a patch that doesn't "change everything" has a better chance of being reviewed and accepted?

Meh :)

Can you think of a way (off the top of your head) to dynamically update those links after a drag and drop?

If I were writing it, I would do the following: On any event (drop, click-to-delete, etc), send the entire new ordering to the server. On the server, do any necessary processing (delete, reorder, whatever). Send back a JSON-encoded result that contains the new ordering as well as any relevant error/success messages. In JS, parse that JSON, and rebuild the list.

Should I practice making a patch file and email it to you (or attach it to an update here, or even on that trac ticket you referenced above) for you to check my progress?

If it's not a complete patch yet (JS but no PHP), just post it as an attachment here. To get the diff:

svn diff > /path/to/your/patch.diff

Once your first rev is done, let's move to BP Trac.

Hi Dom,

Patch is looking good so far. A couple of comments:

1) I'm realizing now that saving-on-drag may not be the best option, simply because it doesn't match the interface (which suggests that Save only happens when you click the Save button). You want to continue to allow people to cancel any changes they've made. So I think you need to stick with saving only on form submit. However, you can still send that AJAX request, and use it to handle the renumbering. Just don't save it until the form is submitted.

2) Don't forget to change the .dev.js enqueue back to .js. Check out the SCRIPT_DEBUG constant, which BP core scripts support: https://codex.wordpress.org/Debugging_in_WordPress#SCRIPT_DEBUG. (You should always have WP_DEBUG enabled when you develop as well.)

3) Make sure that you test with new fields as well as existing ones. The two cases shouldn't be too much different if you're not actually saving the option order via AJAX (as I suggest in (1) above), but you'll still want to make sure it works right.

I'll be working on the ajax today, will let you know if run into any issues with the reordering callback.

I always have WP_DEBUG on when developing on cac, completely forgot to turn it on in this project. I've enabled it and SCRIPT_DEBUG, thanks for that link. I just changed the js enqueue code back to the way it's supposed to be.

Will you be at the WordPressNYC Meetup tonight? It's in Brooklyn:

http://www.meetup.com/WordpressNYC/events/71818702/?a=cr1_grp&rv=cr1

Will you be at the WordPressNYC Meetup tonight?

Nope, can't make it tonight. Have fun.

Hi Dom,

We're making progress here.

1) I found a bug in BP related to default display of options. https://buddypress.trac.wordpress.org/ticket/4363#comment:2 You might want to svn up.

2) On further consideration, I would definitely refactor so that the changes are only stored in the DB when the Save button is hit. That could be implemented in one of two ways. (a) You could remove all the AJAX. Do your reordering in JS (as well as any necessary renumbering). Then, pick up the data as part of the form submit. (b) Keep reordering via AJAX. But on page load, render a hidden admin field that has the "original" ordering. That way, if someone hits Cancel, you can restore the ordering. Option (b) sounds pretty hackish, so I would go ahead with (a) (unless I'm missing something).

3) Looks like BP doesn't put a 'delete' icon on the first item in the list. That doesn't make much sense to me, and makes even less sense when you can reorder the fields, so go ahead and remove that.

4) I'm having a hard time clicking into the fields to change the text - it might be that the sortable script is preventing default actions or something. (I can tab into them.)

5) Yes, add the reordering to all types that have options.

6) Make sure you die() at the end of the AJAX request, or a 0 will be returned from admin-ajax.php. Not a big deal since you're not returning any other info, but it will cause malformed JSON if you do send data back to the browser.

7) Re nonces. You either have to use an existing nonce on the screen, or generate your own. The first argument in check_admin_referer() ('bp_reorder_field_options' in your case) has to correspond to a nonce field created on the front-end:

<?php wp_nonce_field( 'bp_reorder_field_options', '_wp_nonce_reorder_field_options' ) ?>

That should be enough to work on for a while ;)

1) I found a bug in BP related to default display of options. https://buddypress.trac.wordpress.org/ticket/4363#comment:2 You might want to svn up.

Done, seems to work fine on my end now

2) Option (b) sounds pretty hackish, so I would go ahead with (a) (unless I'm missing something).

I agree, (a) seems like the most logical path. That's what I'm focusing on now, refactoring the code - moving from ajax db access to only on save action. JS will now just be for controlling the UI.

Looks like BP doesn't put a 'delete' icon on the first item in the list.

When you say "so go ahead and remove that" do you mean remove the delete functionality all-together? Or just remove the code that prevents a delete link on the first option, so all options get a delete link?

4) I'm having a hard time clicking into the fields to change the text

I'm not experiencing this with the option text fields. I did notice this effect on the group/fields page (the main Profile Fields page). I find that I have to be very careful when clicking a field's Edit button. If I'm not exact, it picks up my click as a drag-sort. Once the profiles are saving and updating correctly I'll dig into ui-sortable code to see if there's an option that affects this.

Dom - This looks really excellent.

The purpose of the delete button on the New Field screen is to remove them from the interface (so it would be DOM manipulation only in this case, no AJAX). The fact that no [x] is included for the first option on the New Field screen seems to me like a bug (not one you created, but one that's always been there) - clearly it's trying to prevent you from attempting to create an "options" field without any options, but I think this could be just as well accomplished with some validation + a message "Create some options" or something like that.

I think that this is ready to be moved over to Trac, where there will be more eyeballs for things like backward compatibility with existing fields, cross-browser testing, and so forth. Can I ask you to post to the BP ticket I listed in the first comment above, with a brief description of what you've done so far? Then I'll chime in with my two cents :)

Patch is up:

http://buddypress.trac.wordpress.org/ticket/3778#comment:1

Sorry for the delay.

Congrats, Dom! Nice!

Thank you both, it really is an amazing feeling.

Boone,

There really wasn't much discussion on trac, I assumed there would be more back and forth on the ticket. I see that @jjj was the one who marked it as fixed, how much testing do you think got done. I know he wouldn't accept something into core without ensuring it worked. Did I just get that close to a final patch on my third attempt?

Did I just get that close to a final patch on my third attempt?

Ha, that's definitely part of it. I'd already signed off on it, and I know that Paul had tested it, and I'm sure John tested it before putting it in. So that's three of us. Since there aren't many members of the BP dev team, we try to conserve our energies by only putting a lot of discussion into tickets where there's bound to be disagreement. But your fix was pretty straghtforward, and the patch was good, so he skipped that discussion :)

Awesome! Congrats, Dom!!

Thank you both! I just hope it works correctly once it's out in the wild!! :-)