WordPress.org

Make WordPress Core

Opened 17 months ago

Closed 17 months ago

Last modified 17 months ago

#22524 closed defect (bug) (fixed)

"Delete" for the media modal

Reported by: nacin Owned by: 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 17 months ago.
22524.patch (1.1 KB) - added by merty 17 months ago.
22524.3.patch (2.1 KB) - added by merty 17 months ago.
This must be the proper way to do it
22524.diff (1.9 KB) - added by nacin 17 months ago.
22524.2.diff (5.1 KB) - added by nacin 17 months ago.
22524.3.diff (5.7 KB) - added by koopersmith 17 months ago.

Download all attachments as: .zip

Change History (20)

comment:1 merty17 months ago

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

comment:2 SergeyBiryukov17 months ago

  • Keywords has-patch added

comment:3 miqrogroove17 months 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.

comment:4 follow-up: miqrogroove17 months 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."

merty17 months ago

merty17 months ago

comment:5 in reply to: ↑ 4 merty17 months 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.

merty17 months ago

This must be the proper way to do it

comment:6 merty17 months 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.

comment:7 koopersmith17 months 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.

nacin17 months ago

nacin17 months ago

comment:8 nacin17 months 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.

comment:9 nacin17 months ago

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

koopersmith17 months ago

comment:10 koopersmith17 months 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.

comment:11 ryan17 months ago

media-models.js needs refresh.

comment:12 ryan17 months 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

comment:13 ryan17 months ago

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

comment:14 SergeyBiryukov17 months ago

#22620 was marked as a duplicate.

Note: See TracTickets for help on using tickets.