xProfileFields - deletion of drop down select box values not possible

[GreenFeetz]

Hi all,

not sure if this is truly a defect, or me just being stupid, but currently I cannot delete values from drop down select box anymore. The [x] behind the value is missing for existing values.

WP: 3.9.1
BP: 2.0.1

[imath]

Hi GreenFeetz

Thanks for your feedback. I confirm it's a regression from 1.9.2. Just tested, as you said in previous versions it was possible to delete an option while editing a profile field. It's no more the case.

i'm suggesting 5647.patch to solve the issue. i think this should be in 2.0 branch.

[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?
• 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 ); ?>"

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.

[imath]

Replying to 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 ​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. We could alternatively check if the $options[$i]->name is not empty.

• 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 depending on your preference about point 1

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.

[imath]

As it's a regression from 1.9, i think we should try to fix this for 2.0.2

[DJPaul]

Didn't mean to move this

[DJPaul]

imath; looks like JJJ accidentally fixed this in r8643

Can you check and consider how best to fix this for the branch?

[djpaul]

In 8657:

xProfile: re-instate profile field option delete button.

It was accidentally removed in the 2.0 release during the xProfile Field Types re-implementation.
Fixes #5647, props imath

[DJPaul]

Decided to fix by reinstating code from the 1.9 branch, via imath's patch. The -1 conditional wasn't required, and this now works the same in the 2.0 branch and 1.9 branch, according to my testing.

[imath]

In 8739:

In case of an xProfile field requiring options (eg: selectbox, radio buttons), make sure the first option doesn't show a delete link when creating or editing this type of fields.

See #5647