WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 2 years ago

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

Download all attachments as: .zip

Change History (20)

comment:1 @merty2 years ago

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

comment:2 @SergeyBiryukov2 years ago

  • Keywords has-patch added

comment:3 @miqrogroove2 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.

comment:4 follow-up: @miqrogroove2 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."

@merty2 years ago

@merty2 years ago

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

@merty2 years ago

This must be the proper way to do it

comment:6 @merty2 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.

comment:7 @koopersmith2 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.

@nacin2 years ago

@nacin2 years ago

comment:8 @nacin2 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.

comment:9 @nacin2 years ago

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

@koopersmith2 years ago

comment:10 @koopersmith2 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.

comment:11 @ryan2 years ago

media-models.js needs refresh.

comment:12 @ryan2 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

comment:13 @ryan2 years ago

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

comment:14 @SergeyBiryukov2 years ago

#22620 was marked as a duplicate.

Note: See TracTickets for help on using tickets.