Make WordPress Core

Opened 3 years ago

Closed 2 years ago

Last modified 2 years ago

#54188 closed defect (bug) (fixed)

Different theme screenshot sizes are displayed in the customizer preview

Reported by: poena's profile poena Owned by: audrasjb's profile audrasjb
Milestone: 6.0 Priority: normal
Severity: normal Version: 3.9
Component: Themes Keywords: has-patch has-testing-info assigned-for-commit commit
Focuses: Cc:

Description (last modified by poena)

On the Add themes page in the WordPress admin you can select "Live Preview" or "Preview" to preview themes in the customizer.

In the preview, the customizer sidebar shows a screenshot of the theme.
This screenshot is displayed in different sizes depending on the aspect ratio of the screenshot that is in the theme folder.

The screenshot size should be enforced and the screenshot should be consistent and displayed in the same size for all themes.

Related meta ticket: https://meta.trac.wordpress.org/ticket/5834

Attachments (3)

customizer-preview-theme-installation.png (72.7 KB) - added by poena 3 years ago.
Different image sizes in the customizer preview
54188.diff (1.1 KB) - added by karpstrucking 3 years ago.
54188.2.diff (1.2 KB) - added by costdev 2 years ago.
Patch refreshed against trunk.

Download all attachments as: .zip

Change History (20)

@poena
3 years ago

Different image sizes in the customizer preview

#1 @poena
3 years ago

  • Description modified (diff)

@karpstrucking
3 years ago

#2 @karpstrucking
3 years ago

  • Keywords has-patch added; needs-patch removed

took a stab at this, basically just a mimic of the approach used on /wp-admin/themes.php

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


2 years ago

#4 @audrasjb
2 years ago

  • Keywords needs-unit-tests needs-testing added
  • Milestone changed from 5.9 to 6.0

As per today's bug scrub, it looks like this patch still needs testing and unit tests. Moving for 6.0 consideration.

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


2 years ago

#6 @hellofromTonya
2 years ago

  • Keywords has-testing-info added

Marking as having has-testing-info as the instructions are in the description and screenshots.

#7 @hellofromTonya
2 years ago

  • Keywords needs-unit-tests removed

As this ticket addresses visual styling to hold the preview within a set sized container, A PHP unit test is not useful for this patch. Why? While it could capture the output buffer to check if the HTML and CSS are expected, it cannot check the result to ensure the changes actually resolve the preview issue.

Removing the keyword.

#8 @Boniu91
2 years ago

@poena Could you tell us what is the 1st theme in the screenshot? Second theme is TwentyTwenty but I can't identify the other one.

#9 @poena
2 years ago

The theme is called c
https://wordpress.org/themes/c/

The linked meta ticket lists a few other themes with different screenshot sizes.

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


2 years ago

@costdev
2 years ago

Patch refreshed against trunk.

#11 @costdev
2 years ago

  • Keywords commit added; needs-testing removed
  • Version set to 3.9

Test Report

Environment

  • Server: Apache (Linux)
  • WordPress: 6.0-beta1-53167-src
  • Browser: Chrome 100.0.4896.88
  • OS: Windows 10
  • Theme: Twenty Twenty-One
  • Plugins: None activated

Steps

  1. Navigate to /wp-admin/theme-install.php?theme=twentytwenty. Note the size of the screenshot
  2. Navigate to /wp-admin/theme-install.php?theme=c. Notice that the screenshot is a different size. ✅
  3. Apply 54188.2.diff.
  4. Repeat steps 1-2 (a hard refresh might be needed). Notice that the screenshots are now the same size. ✅

Results

  1. Issue reproduced. ✅
  2. 54188.2.diff resolves the issue. ✅

Notes

  • Removing needs-testing.
  • Marking for commit consideration.

#12 @audrasjb
2 years ago

  • Keywords assigned-for-commit added
  • Owner set to audrasjb
  • Status changed from new to reviewing

Self assigning for commit.

#13 @audrasjb
2 years ago

  • Keywords needs-refresh added; commit removed

I'm not sure removing the version of the file is intended. Adding a new PR to re-add this for better cache refreshing.

This ticket was mentioned in PR #2599 on WordPress/wordpress-develop by audrasjb.


2 years ago
#14

  • Keywords needs-refresh removed

#15 @audrasjb
2 years ago

  • Keywords commit added

PR added and tests are passing. Committing right now.

#16 @audrasjb
2 years ago

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

In 53191:

Themes: Use consistent thumbnail sizes in Customizer preview screen.

This ensures displaying theme thumbnails in consistent sizes in Customizer theme preview.

Props poena, karpstrucking, costdev, audrasjb.
Fixes #54188.

Note: See TracTickets for help on using tickets.