Project

General

Profile

Feature #1386

Create way to create custom order BP Profile Field checklist items

Added by Matt Gold almost 10 years ago. Updated almost 9 years ago.

Status:
Resolved
Priority name:
Low
Category name:
BuddyPress (misc)
Target version:
Start date:
2011-11-28
Due date:
% Done:

0%

Estimated time:

Description

I was just working on the BP Profile fields on the Commons (for Role). Added a new one -- alumnus/a. Right now, the options for ordering the elements are alpha a-->z, alpha z-->a, and order entered. It would be nice if there was a way to order based on a custom ordering system. I'd say that this should be built into BP rather than a plugin, if possible. In any event, certainly not a big deal, but I mention it because I noticed it.

custom-order-profile-options-1.diff (4.96 KB) custom-order-profile-options-1.diff Dominic Giglio, 2012-07-16 01:11 PM
custom-order-profile-options-2.diff (5.37 KB) custom-order-profile-options-2.diff Dominic Giglio, 2012-07-19 12:47 PM
custom-order-profile-options-3.diff (8.16 KB) custom-order-profile-options-3.diff Dominic Giglio, 2012-07-21 04:30 PM

History

#1 Updated by Boone Gorges almost 10 years ago

  • Status changed from New to Assigned

#2 Updated by Matt Gold almost 10 years ago

Cool. Thanks, Boone.

#3 Updated by Boone Gorges over 9 years ago

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.

#4 Updated by Boone Gorges over 9 years ago

  • Assignee changed from Boone Gorges to Raymond Hoh

Reassigning to Ray, but Dom, you should feel free to grab it if you get to it first :)

#5 Updated by Dominic Giglio over 9 years ago

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. :-)

#6 Updated by Boone Gorges over 9 years ago

  • Assignee changed from Raymond Hoh to Dominic Giglio
  • Target version changed from 1.4 to Future release

Moving out of the milestone for the moment, and reassigning to Dom.

#7 Updated by Dominic Giglio over 9 years ago

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

#8 Updated by Dominic Giglio over 9 years ago

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.

#9 Updated by Dominic Giglio over 9 years ago

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?

#10 Updated by Boone Gorges over 9 years ago

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.

#11 Updated by Dominic Giglio over 9 years ago

I like the idea of sending the updates right when they're reordered. Saves me from having to rename the "Option 1:" labels as well, since that will be handled in the server output.

First diff attached.

#12 Updated by Boone Gorges about 9 years ago

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.

#13 Updated by Dominic Giglio about 9 years ago

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

#14 Updated by Boone Gorges about 9 years ago

Will you be at the WordPressNYC Meetup tonight?

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

#15 Updated by Dominic Giglio about 9 years ago

You didn't miss anything at the meetup, actually would have been a waste of your time. :-)

I've attached my latest diff. I've got the UI talking to the backend and it updates the db with the new custom ordering. I'm have some trouble getting the wpnonce to work (that's why it's commented out). A generic nonce is added to that editing form (somewhere) but when I try and check_admin_referer() on that nonce I get errors. Not sure if I'm supposed to be adding a new nonce to that form the way the group form does?

Right now it is doing the reordering (updating the db) after each drag/drop. Also, it doesn't handle new fields properly. What I'm working on now is using the js that creates new fields to reinitialize the sortable ui code so it picks up the new fields.

Also, there are currently 4 types of fields that have multiple selections: radio, dropdown, multiselect and checkox.

Am I adding this functionality to all 4 types?

Let me know what you think of the current diff, I just want to make sure I'm going in the right direction.

#16 Updated by Boone Gorges about 9 years ago

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 ;)

#17 Updated by Dominic Giglio about 9 years ago

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.

#18 Updated by Dominic Giglio about 9 years ago

Boone,

I've addressed most of what you outlined above. The js is now only responsible for handling the UI. All db operations happen on save. I didn't even have to change anything, BP is just handling the custom order and updating the db accordingly. I think I fixed your 'not able to select text fields' error, but like I said it doesn't happen on Chrome (archlinux), so let me know if you still see the issue. A delete [X] now always shows up, except on the new field page (it's a little hard to come up with a delete link for something that doesn't exist yet). The new fields page should also be behaving the same as the edit fields page.Reordering works for all 4 types of 'multiple' option fields.

I've attached a new diff, let me know how it works when you get a chance to apply it.

#19 Updated by Boone Gorges about 9 years ago

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 :)

#21 Updated by Boone Gorges about 9 years ago

  • Status changed from Assigned to Hold
  • Target version changed from Future release to 1.5
  • Severity set to Low impact

Dom's patch was committed to BuddyPress at https://buddypress.trac.wordpress.org/ticket/3778#comment:5. It'll be included in BP 1.7. I'm putting this ticket into Commons 1.5 with the expectation that BP 1.7 will be out by the time we do Commons 1.5 (or at least we'll have a fairly stable pre-release version available). Let's reassess as 1.5 release gets closer.

Nice job on this one, Dom!

#22 Updated by Matt Gold about 9 years ago

Congrats, Dom! Nice!

#23 Updated by Dominic Giglio about 9 years ago

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?

#24 Updated by Boone Gorges about 9 years ago

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 :)

#25 Updated by Boone Gorges almost 9 years ago

  • Status changed from Hold to Resolved

This has been implemented in BP, and will be part of BP 1.7, which'll likely be available before CAC 1.5. Congrats, Dom!

#26 Updated by Matt Gold almost 9 years ago

Awesome! Congrats, Dom!!

#27 Updated by Dominic Giglio almost 9 years ago

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

Also available in: Atom PDF