Skip to:
Content

BuddyPress.org

Opened 4 months ago

Closed 4 months ago

#8305 closed enhancement (fixed)

Add regular delete confirmation screen for XProfile field groups

Reported by: oztaser Owned by: slaFFik
Milestone: 7.0.0 Priority: normal
Severity: normal Version: 2.3.0
Component: Administration Keywords: has-patch commit
Cc:

Description

It will be good to add a js confirm to the delete link because users can accidentally click.

Attachments (4)

8305.patch (885 bytes) - added by oztaser 4 months ago.
Screenshot_2020-05-23_01-51-43.png (58.2 KB) - added by oztaser 4 months ago.
8305-2.patch (6.3 KB) - added by oztaser 4 months ago.
8305-3.patch (6.3 KB) - added by oztaser 4 months ago.

Download all attachments as: .zip

Change History (12)

@oztaser
4 months ago

#1 @imath
4 months ago

  • Component changed from Extended Profile to Administration
  • Keywords has-patch reporter-feedback added
  • Milestone changed from Awaiting Review to 7.0.0
  • Owner set to slaFFik
  • Version set to 2.3.0

Hi @oztaser

Thanks for this new contribution, I agree 💯. But just as I've commented on your ticket about the activity delete link, could we use a regular confirmation screen instead of the JavaScript confirm ?

Also, what about adding the delete link into the edit xProfile page, just like you suggested for the activity ?

#2 @oztaser
4 months ago

  • Keywords reporter-feedback removed

Hi @imath,

Thanks for your suggestion. I am gonna work on it.

@oztaser
4 months ago

#3 @oztaser
4 months ago

  • Summary changed from Add js confirm to XProfile field group delete link to Add regular confirmation screen for XProfile field groups

I added a regular confirmation screen for field groups and added the delete link into the edit field group page.

#4 @oztaser
4 months ago

  • Summary changed from Add regular confirmation screen for XProfile field groups to Add regular delete confirmation screen for XProfile field groups

#5 @imath
4 months ago

Hi @oztaser

I just had a look at your patch and I've tested it. It's a very interesting work. Thanks a lot.

  1. Suggestion:
// Let's use strict comparison at line 90 like:
in_array( $mode, array( 'delete_group', 'do_delete_group' ), true )
  1. The Base group is required and not deletable

Into the edit group page, you should only include the delete link if the edited group is not the Base one.

  1. What about confirming fields deletion?

I'm wondering if we should also add this confirmation step for fields. Would it be too annoying at usage?

@oztaser
4 months ago

#6 @oztaser
4 months ago

Hi again,

I've updated the patch for your first two suggestions.

What about confirming fields deletion?

I believe accidentally losing data is more annoying than confirmation screen so I think confirmation is always good for user experience.

I also can work on it if you think we should add the screen.

#7 @imath
4 months ago

  • Keywords commit added

Hi @oztaser

Thanks a lot for updating the patch and sorry it took me so long to review it. I believe it's ready to be committed so I'll do it in a few minutes.

I also can work on it if you think we should add the screen.

I think so and that would be awesome!

#8 @imath
4 months ago

  • Resolution set to fixed
  • Status changed from new to closed

In 12660:

xProfile Group Admin: add confirmation screen for the delete action

Props oztaser

Fixes #8305

Note: See TracTickets for help on using tickets.