Make WordPress Core

Opened 8 years ago

Closed 22 months ago

#37165 closed defect (bug) (worksforme)

Problem bulk actions is disabled

Reported by: sebastianpisula's profile sebastian.pisula Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Quick/Bulk Edit Keywords: has-screenshots has-patch close
Focuses: ui Cc:

Description

I have this filter: add_filter( 'bulk_actions-{$this->screen->id}', '__return_empty_array' ); and in page I have space because I have empty div:

<div class="alignleft actions bulkactions"></div>

Problem is in wp-admin/includes/class-wp-list-table.php:1182:

<?php
 if ( $this->has_items() ): ?>
                <div class="alignleft actions bulkactions">
                        <?php $this->bulk_actions( $which ); ?>
                </div>
                <?php endif;

Attachments (4)

space.png (2.8 KB) - added by sebastian.pisula 8 years ago.
37165.diff (1.1 KB) - added by andizer 8 years ago.
37165.2.diff (1.2 KB) - added by mariovalney 6 years ago.
wp 5.0 and now.png (99.2 KB) - added by afercia 22 months ago.

Download all attachments as: .zip

Change History (16)

#1 @afercia
8 years ago

  • Component changed from General to Administration
  • Focuses ui added
  • Keywords has-screenshots needs-patch added
  • Milestone changed from Awaiting Review to Future Release

Ideally empty elements shouldn't be printed out, worth investigating what can be done on the PHP side. Alternatively a CSS-only solution could work.

#2 @SergeyBiryukov
8 years ago

  • Component changed from Administration to Quick/Bulk Edit

@andizer
8 years ago

#3 @andizer
8 years ago

Just added a patch to solve this. Feedback is welcome.

#4 @andizer
8 years ago

  • Keywords has-patch added; needs-patch removed

#5 @andizer
8 years ago

  • Keywords needs-testing added

#6 @flixos90
8 years ago

Hi @andizer, the patch looks good in my opinion as far as it fixes the issue. :)
We might need to be careful with the change since when the bulk_actions() method is overridden in a subclass, the change could lead to issues since the wrapping div would be gone then. On the other hand, I think it's rather unlikely to override this method since one should preferably use get_bulk_actions() for that purpose, so I tend to say we're fine applying this change. I'm just thinking out loud here to make sure we consider this possibility.

#7 @afercia
8 years ago

Even if the bulk actions div is not printed out, the wrapper .tablenav has always a fixed height of 30px, so the empty space will still be there, unless I'm missing something. .tablenav can also contain the pagination links and the number of items, not to mention additional filters.

@mariovalney
6 years ago

#10 @mariovalney
6 years ago

  • Keywords bulk-reopened added; needs-testing removed

To keep tabnav structure inside display_tablenav() method, I guess we can remove the empty space created and add a CSS :empty rule to display none this section.

I added 37165.2.diff with this in mind.

#11 @webcommsat
22 months ago

  • Keywords needs-testing added

Component review today:

  • there is a patch that was designed to address the comments on the previous patch. @mariovalney were they any updates to this patch or comments you wanted to raise before further testing of the patch?

@afercia and @flixos90 do you have any comments to add to help move this along?

@oglekler and @webcommsat will also be testing the patch in the coming fortnight.

#12 @webcommsat
22 months ago

(Trac posted twice, showing first had an error. So this comment can be ignored)

Last edited 22 months ago by webcommsat (previous) (diff)

#13 @afercia
22 months ago

  • Keywords close added; bulk-reopened needs-testing removed

@webcommsat thanks for looking into this old ticket.

I think this is not an issue any longer. When using the filter as described above to remove the bulk actions entirely, that empty space is no longer there. I think this changed in [46866] by removing the 2px vertical padding so that the empty container doesn't take space on the page, even though it does have some horizontal padding. See attached screenshots:

  • At the top: WordPress 5.0.
  • At the bottom: current trunk.

#14 @webcommsat
22 months ago

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

I have reviewed this with a colleague today, and we get the same as reported by @afercia above (thank you for testing this).

I have marked this as works for me and it can be closed. Thank you for all the comments on this ticket.

Note: See TracTickets for help on using tickets.