Make WordPress Core

Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#22524 closed defect (bug) (fixed)

"Delete" for the media modal

Reported by: nacin's profile nacin Owned by: ryan's profile ryan
Milestone: 3.5 Priority: normal
Severity: normal Version:
Component: Media Keywords: has-patch commit
Focuses: Cc:

Description

Good to have: "Delete" actually deletes the attachment.

Less important but still nice to have: "Edit" opens post.php in a new screen.

Attachments (6)

22524.2.patch (1.1 KB) - added by merty 11 years ago.
22524.patch (1.1 KB) - added by merty 11 years ago.
22524.3.patch (2.1 KB) - added by merty 11 years ago.
This must be the proper way to do it
22524.diff (1.9 KB) - added by nacin 11 years ago.
22524.2.diff (5.1 KB) - added by nacin 11 years ago.
22524.3.diff (5.7 KB) - added by koopersmith 11 years ago.

Download all attachments as: .zip

Change History (20)

#1 @merty
11 years ago

This seems to be the only way but naturally, & is being replaced with &

#2 @SergeyBiryukov
11 years ago

  • Keywords has-patch added

#3 @miqrogroove
11 years ago

I have concerns about this patch. It doesn't include the JS confirmation prompt or any kind of AYS feature. It also seems strange to navigate away from a modal window just to delete one image.

#4 follow-up: @miqrogroove
11 years ago

  • Keywords has-patch removed

Patch is broken anyway. It sends me to

/wp-admin/post.php?action=delete&post=118&_wpnonce=f731a7d735

and displays "Please try again."

@merty
11 years ago

@merty
11 years ago

#5 in reply to: ↑ 4 @merty
11 years ago

Replying to miqrogroove:

I have concerns about this patch. It doesn't include the JS confirmation prompt or any kind of AYS feature. It also seems strange to navigate away from a modal window just to delete one image.

You're right, added the prompt.

Replying to miqrogroove:

Patch is broken anyway. It sends me to

/wp-admin/post.php?action=delete&post=118&_wpnonce=f731a7d735

and displays "Please try again."

See my first comment above: "This seems to be the only way but naturally, & is being replaced with &"

This is what can be done if we just use what we currently have. The correct way of doing the deletion is to write a JS function to handle it which triggers the deletion and removes the item from the grid, without navigating away from the modal window.

@merty
11 years ago

This must be the proper way to do it

#6 @merty
11 years ago

Just figured out the way the media modal works, sorry for the mess :)

22524.3.patch implements both of the functionalities mentioned, except it does not remove it from the grid. Will add that as well, once I figure out the best way to do it.

#7 @koopersmith
11 years ago

  • Keywords needs-refresh added

We should handle the deletion via an ajax request. Editing should be handled through a link with target="_blank" and the link should be properly processed in the PHP, not the JS.

@nacin
11 years ago

@nacin
11 years ago

#8 @nacin
11 years ago

  • Keywords has-patch added; needs-refresh removed

No one has been clamoring for image editing. Since we would be opening a new window but not refreshing the attachment details in the open modal in the original window, let's leave "Edit" for when image editing it is integrated into the dialog in a future release.

Attached patch implements Delete Permanently and also makes the save-attachment nonces attachment-specific. The link doesn't quite line up with the dimensions, that's about the only issue.

#9 @nacin
11 years ago

  • Priority changed from low to normal
  • Summary changed from "Edit" and "Delete" for the media modal to "Delete" for the media modal

@koopersmith
11 years ago

#10 @koopersmith
11 years ago

  • Keywords commit added

22524.3.diff builds off of 22524.2.diff:

  • Adds CSS to align the delete link.
  • Only shows the delete link when the attachment has uploaded. This prevents a race condition between the attachment removal and the upload request.
  • Prevents get/save/delete attachment requests from running if the attachment doesn't have an id.

#11 @ryan
11 years ago

media-models.js needs refresh.

#12 @ryan
11 years ago

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

In 22869:

Add a delete link to the media modal.

Props merty, nacin, koopersmith
fixes #22524

#13 @ryan
11 years ago

Refreshed media-models.js to resolve patch conflict with [22865]. Double check that work.

#14 @SergeyBiryukov
11 years ago

#22620 was marked as a duplicate.

Note: See TracTickets for help on using tickets.