[buddypress-trac] [BuddyPress Trac] #5647: xProfileFields - deletion of drop down select box values not possible

[buddypress-trac]

#5647: xProfileFields - deletion of drop down select box values not possible
-----------------------------------+------------------------------
 Reporter:  GreenFeetz             |       Owner:
     Type:  defect (bug)           |      Status:  new
 Priority:  normal                 |   Milestone:  Awaiting Review
Component:  XProfile               |     Version:  2.0
 Severity:  normal                 |  Resolution:
 Keywords:  has-patch 2nd-opinion  |
-----------------------------------+------------------------------

Comment (by imath):

 Replying to [comment:2 DJPaul]:
 > Some feedback on the patch.
 >
 > * `-1` appears to be a magic number. What is -1? Why don't we want to
 render this bit of the template for -1? It also appears to be not in the
 1.9.2 version; did I change the logic in the 2.0 version that made this
 extra check necessary?

 I think you did change the logic see line
 [https://buddypress.trac.wordpress.org/browser/trunk/src/bp-xprofile/bp-
 xprofile-classes.php#L2811 2811] of bp-xprofile-classes.php. In the
 comment above you're indicating :
 {{{
 // If there are still no children options set, this must be the "new
 field" screen, so add one new/empty option.
 }}}

 So i thought, maybe wrongly, that -1 means it's a  new field and we
 shouldn't delete an option that doesnot exist, like it was the case in
 1.9.2 for instance.

 > * For `id="delete-<?php echo esc_attr( $options[$i]->id ); ?>"`, it's
 neater to concatenate `delete-` with the variable inside the `esc_attr()`
 call (similar how to you built the `href` value), e.g. `id="<?php echo
 esc_attr( 'delete-' . $options[$i]->id ); ?>"`

 Yeah, i've just copied the 1.9.2 line, sorry for the shortcut. I'll work
 on a new patch if you think the "-1" check is the right move, we could
 alternatively check if the $options[$i]->name is not empty

 > While `[x]` is a lousy, untranslatable way of indicating a delete button
 (which is the fault of an old version of BP), I don't think we need to
 adjust that for this ticket, but we should make another ticket to go
 through and replace ticks/crosses with genericons or dashicons or
 something similar.
 I agree the trash dashicon for instance.

--
Ticket URL: <https://buddypress.trac.wordpress.org/ticket/5647#comment:3>
BuddyPress Trac <http://buddypress.org/>
BuddyPress Trac