Make WordPress Core

Opened 6 weeks ago

Closed 6 weeks ago

#65732 closed defect (bug) (fixed)

Media grid: Fix toolbar height and 'button-link' styling when Bulk editing

Reported by: afercia Owned by: afercia
Priority: normal Milestone: 7.1
Component: Media Version: 7.0
Severity: normal Keywords: has-screenshots has-patch has-test-info commit
Cc: Focuses: ui, accessibility

Description

In the media grid, when pressing 'Bulk select', the toolbar switches to another mode and shows alternative buttons.

After the admin reskin in WordPress 7.0, these buttons are taller than expected. After switching, the toolbar is noticeably taller thus triggering a 'jump' in the page content that isn't a great user experience. See attached video recording.

WordPress 6.9 was not perfect either: the alternative buttons height was 32px vs. the standard height of 30px. The triggered 'jump' was only by 2 pixels and barely noticeable. Now, it's by 8 pixels, as the buttons height changes from 32px to 40px.

Additionally, when the 'Media trash' is enabled, the toolbar provides another mode with two more buttons:

  • Restore from Trash
  • Delete permanently

The height of these buttons should be fixed as well.
More importantly, the 'Delete permanently' button should have the 'button-link' styling: no border, no background. The button text should be red.
That's is currently broken, and it's a regression from 7.0 as the styles for the default button-link have higher specificity after the admin reskin.

To reproduce:

  • Go to the Media Library, in grid mode.
  • Click 'Bulk select'.
  • Observe the toolbar height changes and triggers a jump in the page content.
  • Add define( 'MEDIA_TRASH', true ); to your config.php to enable the Media trash.
  • Click 'Bulk select', then select an attachment and click 'Move to Trasg'.
  • Select 'Trash' from the 'Filter by type' select.
  • The media grid will now show the trashed attachment.
  • Click 'Bulk select': the 'Restore from Trash' and 'Delete permanently' buttons will appear.
  • Observe these two buttons are taller than expected and will trigger the same jump in the page content described above.
  • Observe the 'Delete permanently' is not styled as a button-link and its text is not red.
  • Select the trashed attachment.
  • The two buttons will now be enabled.

-in the page content described above.

  • Observe the 'Delete permanently' is still not styled as a button-link and its text is not red.

Attachments (10)

00 toolbar 7.0.png (46.1 KB ) - added by afercia 6 weeks ago.
The toolbar buttons different height in 7.0
02 toolbar modes 6.9.png (43.2 KB ) - added by afercia 6 weeks ago.
The toolbar buttons height in 6.9
03 delete permanently.png (111.6 KB ) - added by afercia 6 weeks ago.
The Delete permanently button in 6.9 and 7.0.
65732-trunk-before-bulk-select.jpg (42.3 KB ) - added by khokansardar 6 weeks ago.
Before (trunk): Bulk select mode - toolbar 86px, buttons 40px, 6px jump in page content.
65732-trunk-before-delete-permanently.jpg (43.4 KB ) - added by khokansardar 6 weeks ago.
Before (trunk): Trash mode - 'Delete permanently' is theme blue with a border, not a red button-link.
65732-patch-media-toolbar-fixed.jpg (42.5 KB ) - added by khokansardar 6 weeks ago.
After (PR 12714): buttons 32px and no jump, 'Delete permanently' correctly red and borderless - but 'Restore from Trash' has lost its filled primary styling.
65732-patch-primary-button-regression.jpg (57.6 KB ) - added by khokansardar 6 weeks ago.
Regression with PR 12714: Settings > General 'Save Changes' renders as an outlined secondary button instead of the filled primary button.
before-all.jpg (166.4 KB ) - added by ozgursar 6 weeks ago.
Before patch 12714
after-all.jpg (286.7 KB ) - added by ozgursar 6 weeks ago.
After patch 12714
spinner.png (264.2 KB ) - added by afercia 6 weeks ago.

Download all attachments as: .zip

Change History (28)

@afercia
6 weeks ago

The toolbar buttons different height in 7.0

@afercia
6 weeks ago

The toolbar buttons height in 6.9

@afercia
6 weeks ago

The Delete permanently button in 6.9 and 7.0.

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


6 weeks ago
#1

  • Keywords has-patch added

Trac ticket: https://core.trac.wordpress.org/ticket/65732

Fixes the height and styling of the various buttons in the Media grid.
Avoids a vertical 'jump' in the page content.

## Use of AI Tools
None

Video recording to illustrate the 'jump' in the page content.

https://github.com/user-attachments/assets/5eeb569c-10f2-4025-abc3-293bb37c265a

#2 @afercia
6 weeks ago

I uploaded the video recording to illustrate the 'jump' in the page content to the linked PR, as the file exceeds the size limit in Trac.

#3 @khokansardar
6 weeks ago

  • Focuses ui added

#4 @joedolson
6 weeks ago

  • Milestone Awaiting Review7.1

Since this is continuing fall out from the 7.0 design refresh and the changes are minor in scope, I'm going to milestone this for 7.1. It would be preferable to get design polish done quickly following 7.0.

#5 @joedolson
6 weeks ago

  • Owner set to joedolson
  • Status newaccepted

This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.


6 weeks ago

@khokansardar
6 weeks ago

Before (trunk): Bulk select mode - toolbar 86px, buttons 40px, 6px jump in page content.

@khokansardar
6 weeks ago

Before (trunk): Trash mode - 'Delete permanently' is theme blue with a border, not a red button-link.

@khokansardar
6 weeks ago

After (PR 12714): buttons 32px and no jump, 'Delete permanently' correctly red and borderless - but 'Restore from Trash' has lost its filled primary styling.

@khokansardar
6 weeks ago

Regression with PR 12714: Settings > General 'Save Changes' renders as an outlined secondary button instead of the filled primary button.

#7 @khokansardar
6 weeks ago

Patch testing report

Patch / PR tested

Environment

WordPress: 7.1-beta3-62828-src
PHP: 8.2.18 (Docker)
MySQL: 8.0.36
OS: macOS 26.5.2
Browser: Chromium, desktop viewport (1512px)
Admin color scheme: Modern
Local wordpress-develop @ http://localhost:8889

Steps

  1. Go to the Media Library in grid mode, measure the toolbar height and the top offset of the attachments grid.
  2. Click 'Bulk select' and measure both again to quantify the vertical 'jump'.
  3. Add define( 'MEDIA_TRASH', true );, move an attachment to the Trash, then select 'Trash' in the 'Filter by type' select.
  4. Click 'Bulk select' and select the trashed attachment so 'Restore from Trash' and 'Delete permanently' are enabled.
  5. Record the computed height, color, background and border of every visible toolbar button.
  6. Repeat 1-5 with the PR applied (hard reload to defeat asset caching).
  7. Regression sweep: check .button-primary elements outside the Media Library (Settings > General, Users > Add New).
  8. Isolate causation by reverting only the .button:not(.button-link) selector via CSSOM and re-measuring.
  9. Run grunt jshint:corejs; grep the QUnit suite for the CSS classes the patch stops emitting.

Results

  • Reproduce on trunk, default mode: pass — toolbar 80px -> 86px on 'Bulk select', grid top 192 -> 198 (6px jump); buttons 32px -> 40px. See 65732-trunk-before-bulk-select.jpg.
  • Reproduce on trunk, Trash mode: pass — 'Delete permanently' computed color: rgb(56,88,233) (theme blue, not red) with a 1px blue border, i.e. not rendered as a button-link. Both reported defects confirmed. See 65732-trunk-before-delete-permanently.jpg.
  • Toolbar jump with patch: pass — toolbar stays 80px and grid top stays 192 in both modes; jump is 0px.
  • Button heights with patch: pass — 'Move to Trash', 'Cancel', 'Restore from Trash' and 'Delete permanently' all render at 32px, matching the default mode.
  • 'Delete permanently' styling with patch: pass — computed color: rgb(204,24,24), background: transparent, border-width: 0, underlined. Correct button-link-delete appearance. See 65732-patch-media-toolbar-fixed.jpg.
  • min-height: 80px is load-bearing: pass — neutralising only that declaration leaves a residual -2px jump (80px -> 78px), so it is doing real work rather than being decorative.
  • Primary buttons across the whole admin: fail — with the patch applied, <input class="button button-primary"> computes background: rgba(0,0,0,0) / color: rgb(56,88,233) / 1px theme border instead of the trunk values background: rgb(56,88,233) / color: rgb(255,255,255). Verified on Settings > General ('Save Changes'), Users > Add New ('Add User'), and inside the patched toolbar itself ('Restore from Trash', 'Move to Trash'). Every primary button in wp-admin renders as a secondary/outlined button. See 65732-patch-primary-button-regression.jpg.
  • Cause isolated: pass — reverting only :not(.button-link) back to .button via CSSOM, with every other hunk left in place, restores the filled primary styling. No other hunk contributes.
  • Re-render side effect of this.model.set() inside render(): fail (non-blocking) — Button.initialize() binds this.listenTo( this.model, 'change', this.render ), so setting size during render fires a nested re-render. Reproduced against the real wp.media.view.Button: the PR's approach runs render twice, the _.defaults() idiom runs it once with identical output.
  • grunt jshint:corejs: pass — 98 media files lint free.
  • QUnit fixtures: pass — no test references select-mode-toggle-button, delete-selected-button, or button-compact, so nothing in the suite breaks.
  • Mobile/narrow viewport: not tested — viewport resizing was unavailable in this session. See the note below.

Conclusion
PR #12714 does fix everything the ticket asks for: the 6px toolbar jump is gone, all Bulk-edit buttons render at the standard 32px, and 'Delete permanently' is correctly restored to a red, borderless button-link. Four of the five files are properly scoped to the Media grid.

However this cannot be committed as-is. The change in src/wp-admin/css/colors/_admin.scss from .button to .button:not(.button-link) is a global admin-wide regression, not a Media change. It raises that rule's specificity from (0,2,0) to (0,3,0), so in the compiled colors.css it now outranks .wp-core-ui .button-primary at (0,2,0) (line 146 vs line 212 of colors/modern/colors.css). Since get_submit_button() always emits class="button button-primary", every primary button in wp-admin loses its filled background and renders as a secondary button. The patched toolbar's own 'Restore from Trash' button is affected too.

A scoped alternative was tested and works: leave the global .button rule untouched and instead add the reset at button-link specificity, e.g.

.wp-core-ui .button.button-link {
	background: transparent;
	border: 0;
	box-shadow: none;
	border-radius: 0;
}

With that substituted for the :not() hunk, 'Delete permanently' keeps color: rgb(204,24,24) / border-width: 0 while 'Restore from Trash' returns to background: rgb(56,88,233) / color: rgb(255,255,255). This also matches the selector pattern already used at the top of the same file (.wp-core-ui .button-link, .wp-core-ui .button.button-link).

The .button-link-delete.button-link-delete doubling is genuinely required and is safe: even after the .button-link reset, .wp-core-ui .button.button-link at (0,3,0) still beats .wp-core-ui .button-link-delete at (0,2,0). Every other core consumer of button-link-delete (widgets, customizer, nav menus) uses class="button-link button-link-delete" without .button, so the doubled selector is a no-op for them and changes only the Media grid button.

Smaller points, none of them blocking:

  • delete-selected.js sets the model inside render(), which triggers the nested re-render described above. _.defaults( this.options, { size: '' } ); in initialize() is the idiom already used in select-mode-toggle.js and avoids it. Because DeleteSelectedPermanentlyButton.initialize() chains through DeleteSelectedButton.prototype.initialize(), that one line would also cover the permanently-delete button and make the browser.js hunk unnecessary — one line in one file instead of two changes in two files.
  • After this patch size is '' in both branches of toggleBulkEditHandler() and '' is already the initialize() default, so both size: '' assignments are now dead and could be dropped.
  • min-height: 80px is a magic number calibrated to the desktop natural height. It is load-bearing today, but at max-width: 782px the toolbar selects and search input grow to min-height: 40px, so the floor is inert there and the jump may well persist on small screens. Worth confirming on a narrow viewport before this lands — I was not able to test that here.
  • The two new comments read "Needs higher specificity" and "Need higher specificity"; worth making them consistent.
  • Minor public-facing DOM change: button-compact and button-large are no longer emitted on Media grid toolbar buttons, so any plugin CSS keyed to .media-toolbar .button-large is affected. Low risk and reasonable, but it is a behaviour change.

The branch is 16 commits behind trunk, but no trunk commit since the branch point touches any of the five files, so it still applies cleanly. No PHP is touched, so there is no escaping, sanitisation, capability, nonce or $wpdb surface, and no function signature, hook parameter or return shape changes.

Please replace the .button:not(.button-link) hunk with a .button-link-scoped rule and re-test the primary buttons; with that change I would be happy to see this committed.

#8 @khokansardar
6 weeks ago

  • Keywords has-test-info added

@shailu25 commented on PR #12714:


6 weeks ago
#9

  • This PR affects the Customize button text, making it difficult to read.

https://github.com/user-attachments/assets/f2fb6700-16e1-4a75-bd3a-8a7a1a4d36af

This ticket was mentioned in Slack in #core-test by noruzzaman. View the logs.


6 weeks ago

@ozgursar
6 weeks ago

Before patch 12714

@ozgursar
6 weeks ago

After patch 12714

#11 @ozgursar
6 weeks ago

Test Report

Patch tested: https://github.com/WordPress/wordpress-develop/pull/12714

Environment

  • WordPress: 7.1-beta4-62899-src
  • Subdirectory: No
  • PHP: 8.4.23
  • Server: nginx/1.29.4
  • Database: mysqli (Server: 8.4.7 / Client: mysqlnd 8.4.23)
  • Browser: Chrome 150.0.0.0
  • OS: macOS
  • Theme: Twenty Twenty-Five 1.5
  • MU Plugins:
    • docker-loopback-fix.php
  • Plugins:
    • Test Reports 1.3.0

Steps taken

  1. Add define( 'MEDIA_TRASH', true ); to wp-config.php
  2. Go to Media Library and click Bulk Select
  3. Observe changed button height causing layout shift
  4. Select an item and move to trash
  5. Go to Trash from filters
  6. Observe the button styles
  7. Apply the patch and repeat steps
  8. Observe button heights corrected to 32px on the toolbar, no layout shift
  9. Observe corrected button styles
  10. Go to other dashboard pages where primary button is rendered. Observe incorrectly styled primary button on General>Settings
  11. ✅ Patch is partially solving the problem but causing primary buttons styled incorrectly on other pages.

Expected result

  • Patch should not affect other button styles such as the Save Changes button on General>Settings

Additional Notes

  • Patch needs to target only the incorrectly styled buttons on the Media Toolbar

Screenshots/Screencast with results

Before
https://core.trac.wordpress.org/raw-attachment/ticket/65732/before-all.jpg

After
https://core.trac.wordpress.org/raw-attachment/ticket/65732/after-all.jpg

@joedolson commented on PR #12714:


6 weeks ago
#12

I hadn't previously notice https://core.trac.wordpress.org/ticket/65697, so I'll withdraw my comments about the padding; those can be handled on that ticket.

@afercia commented on PR #12714:


6 weeks ago
#13

One of the problems on this PR is that, at least for me, running npm run dev (the 'watch' task) doesn't build the CSS drom _admin.scss on the fly as I would have expected. So for any change there I have to stop npm run dev and start it again to see the change. Annoying.

The problem with the CSS specificity is that this:
.button:not(.button-link)
was increasing the specificity, impacting other buttons like the 'Customize' one in the themes page.
I changed it to:
.button:where(:not(.button-link))
which does not increase specificity and should solve the issue.

I also addressed some Copilot feedback.

#14 @afercia
6 weeks ago

All the following issues should now be solved:

  • Settings > General 'Save Changes' renders as an outlined secondary button instead of the filled primary button.
  • Primary buttons across the whole admin: fail
  • verified on
    • Settings > General ('Save Changes')
    • Users > Add New ('Add User')
    • and inside the patched toolbar itself ('Restore from Trash', 'Move to Trash').
  • Re-render side effect of this.model.set() inside render(): fail

On the responsive view the toolbar gets a different height depending on the viewport width and how the various elements within it reflow and wrap into the following row so there will always be a 'jump' there that can't be addressed.

Rather, on the responsive view I see an issue with the spinner that is unrelated to this PR and can be reproduced also on trunk. Is there already a ticket for that?

In the responsive view, the spinner is rendered as a sort of always visible gray horizontal 'border'. See screenshot.
On desktop, it is misaligned: it still uses absolute positionin, which seems unnecessary now that these elements are flex items.

@afercia
6 weeks ago

@afercia commented on PR #12714:


6 weeks ago
#15

Looks like aria-disabled buttons still have red text in this PR. In part this is a problem also on trunk, where aria-disabled buttons have a blue border. See screenshot.
Last commit should fix it.

https://github.com/user-attachments/assets/d5748e44-22d6-4d03-803c-1fe1b86980ed

#16 @joedolson
6 weeks ago

  • Keywords commit added

#17 @afercia
6 weeks ago

  • Owner changed from joedolson to afercia
  • Status acceptedassigned

#18 @afercia
6 weeks ago

  • Resolutionfixed
  • Status assignedclosed

In 62959:

Media: Fix Media grid toolbar height and 'button-link' styling when Bulk editing.

Updates Media Library grid toolbar styling to prevent content shifting and normalizes button appearance when switching modes (e.g., Bulk Select mode) by stabilizing toolbar height and removing unintended button sizing.
Also, adjusts the buttons styling so that 'button-links' for destructive actions use the intended red color and prevents them from being styled as standard buttons.

Developed in https://github.com/WordPress/wordpress-develop/pull/12714

Props afercia, joedolson, shailu25, khokansardar, ozgursar.
Fixes #65732.

Note: See TracTickets for help on using tickets.