Make WordPress Core

Opened 20 months ago

Closed 17 months ago

Last modified 17 months ago

#57890 closed defect (bug) (fixed)

Copy URL and Download Media List Table row actions don't appear when the "Unattached" filter is applied

Reported by: pbiron's profile pbiron Owned by: pbiron's profile pbiron
Milestone: 6.3 Priority: normal
Severity: normal Version: 6.0
Component: Media Keywords: has-patch needs-testing
Focuses: ui, administration Cc:

Description

Between 6.2 Beta 5 and RC1, @bobbingwide noticed that the new "Download" row action doesn't appear when the "Unattached" filter is applied. That row action was added in #57574.

While confirming that to be the case, I also discovered that the same problem exists with the "Copy URL" row action added in #54426 (in WP 6.0).

I'll add a patch when I get a chance.

Change History (11)

#1 @pbiron
20 months ago

  • Owner set to pbiron
  • Status changed from new to assigned

This ticket was mentioned in PR #4202 on WordPress/wordpress-develop by @kebbet.


20 months ago
#2

  • Keywords has-patch added

#3 @kebbet
20 months ago

  • Keywords needs-testing added
  • Version changed from trunk to 6.0

#5 follow-up: @pbiron
20 months ago

PR 4203 is similar to the one added by @kebbet. It has 2 differences:

  1. it doesn't try to remove the redundancy of having the View row action specified twice (once when the Unattached filter is applied and once when any other filters are applied)
    • this maintains the existing order of the other row actions
    • we might want to consider in a future ticket trying to remove the redundancy of adding row actions in the if ( $this->detached ) and else logic, but I don't think this ticket is the proper time to do that
  2. it displays the 'Download file' row action even when the Trash filter is applied...thus giving users one more chance to download the media file before they delete it

#6 @kebbet
20 months ago

Thanks for the similar PR @pbiron. Your approach looks better than the one I added. Looking at the code for the View-action, I'm wondering if adding a check for wp_get_attachment_url( $post->ID ) would make sence, just like view checks for if ( get_permalink( $post->ID ) ).

So something like this:

<?php
        if ( wp_get_attachment_url( $post->ID ) ) {
                $actions['download'] = sprintf(
                        '<a href="%s" aria-label="%s" download>%s</a>',
                        esc_url( wp_get_attachment_url( $post->ID ) ),
                        /* translators: %s: Attachment title. */
                        esc_attr( sprintf( __( 'Download &#8220;%s&#8221;' ), $att_title ) ),
                        __( 'Download file' )
                );
        }

#7 @kebbet
20 months ago

I just edited PR 4202, and it extends PR 4203 with the check for wp_get_attachment_url( $post->ID ).

#8 in reply to: ↑ 5 @kebbet
20 months ago

Replying to pbiron:

PR 4203 is similar to the one added by @kebbet. It has 2 differences:

  1. it doesn't try to remove the redundancy of having the View row action specified twice (once when the Unattached filter is applied and once when any other filters are applied)
    • this maintains the existing order of the other row actions
    • we might want to consider in a future ticket trying to remove the redundancy of adding row actions in the if ( $this->detached ) and else logic, but I don't think this ticket is the proper time to do that
  2. it displays the 'Download file' row action even when the Trash filter is applied...thus giving users one more chance to download the media file before they delete it

Future ticket adressing the redundancy: #57893

#9 @SergeyBiryukov
17 months ago

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

In 55949:

Media: Display the “Copy URL” and “Download file” row actions when the “Unattached” filter is applied.

Due to partially duplicated logic for displaying row actions in the Media Library with and without the “Unattached” filter, the “Copy URL” and “Download file” row actions were unintentionally missing with the filter applied.

This commit aims to simplify the logic and bring more consistency to the code.

Includes displaying the “Download file” row action even when the “Trash” filter is applied, giving the user one more chance to download the media file before they delete it.

Follow-up to [8901], [13100], [16227], [16229], [52842], [55198], [55221].

Props kebbet, costdev, pbiron, oglekler, SergeyBiryukov.
Fixes #57890, #57893.

#10 @SergeyBiryukov
17 months ago

  • Milestone changed from Awaiting Review to 6.3

@SergeyBiryukov commented on PR #4203:


17 months ago
#11

Thanks for the PR! Merged as part of the changes in r55949.

Note: See TracTickets for help on using tickets.