Opened 13 years ago
Closed 12 years ago
#21100 closed defect (bug) (fixed)
Uploading Header Images that are too small
Reported by: | MadtownLems | Owned by: | ryan |
---|---|---|---|
Milestone: | 3.5 | Priority: | normal |
Severity: | minor | Version: | |
Component: | Themes | Keywords: | has-patch commit |
Focuses: | Cc: |
Description
Prior to 3.4, if you uploaded a Header Image that was too small, you'd be prompted to choose a region with the correct aspect ratio and then WordPress would scale the region to fit - resulting in a slightly pixelated image that at least fit the entire space.
With 3.4, a button is introduced to "Skip Cropping, Publish Image as Is." In my experiences, this keeps leading to much worse looking images than cropping and getting a little pixelation.
Here is an example of a duck outside my building:
http://imgur.com/a/hTwiN
When you select a crop region, yes, the image gets slightly distorted, but at least it fills the entire space.
When you publish 'as is', you don't get to choose what gets cropped where, and it doesn't even fill the whole space.
For reference, this theme has a header are of 960x190 and I was working with an image sized at 800x534.
In #wordpress-dev, Sergey suggested displaying a warning when uploading an image size that is too small - which I think is a fantastic idea. Something like:
"Your theme wants a header image of 960 wide x 190 high. The image you've selected is only 800 wide. You can proceed with the image, but it may result in a distorted header image on your site. Consider finding a larger image to use as your header."
I think displaying such a warning would be good customer service for users who don't know much about image sizes or working with images. Ideally, though, as a Network Admin, I'd actually love the ability to disallow selecting too small of images as Header Images completely.
Attachments (3)
Change History (17)
#2
@
13 years ago
Ah sorry - I meant to clarify this in my initial post and then totally blanked on it...
We are _not_ using flexible image heights here.
(And yes, that's intentional. We want to maintain a relatively consistent look/feel across dozens of sites and give Site Admins some control over their header, but not let them alter the size.)
#3
@
13 years ago
So, that changes things, a bit. If you are overriding the feature, then the option to skip cropping and publish as is doesn't make any sense. I would suggest writing a plugin to remove the 'skip cropping' button if the active theme doesn't support flex headers, and then turn that into a patch (that way you don't have to wait until the end of the year for it).
#4
@
13 years ago
If you are overriding the feature
Just to clarify, is choosing not to use a flexible height header considered 'overriding' a feature? I'd suggest that add_theme_support takes it as an optional parameter for a reason: to have it be optional. However, displaying the "publish as is" button all the time seems to suggest that all themes are *expected* to use flexible height headers.
Yes, for a quick solution for us I'll probably hide the "Publish without Cropping" button from Site Admins using css, (since I don't see a way to hook into that part of the upload process) but, at least to me, it seems like core is inconsistent about if it expects all themes to support flexible height headers.
#5
@
13 years ago
Tested in trunk with Twenty Eleven and flex-height argument removed, the picture still gets blown up...
What Theme are you using?
#6
follow-up:
↓ 7
@
13 years ago
@Kobenland:
To confirm, you removed the flex-height argument from functions.php, just setting 'flex-height' to false in the argument array?
I'm confused now. Your screenshot suggests (and my testing verifies) that, after specifically removing the ability for the header image to be a flexible height... you were able to publish a ginormous image as the header image. Shouldn't you be forced to choose a section of the image with an appropriate aspect ratio and have it resized to the pre-determined header image size of 1000x288?
#7
in reply to:
↑ 6
@
13 years ago
Replying to MadtownLems:
To confirm, you removed the flex-height argument from functions.php, just setting 'flex-height' to false in the argument array?
No, I commented the line out
Shouldn't you be forced to choose a section of the image with an appropriate aspect ratio and have it resized to the pre-determined header image size of 1000x288?
I guess that would be something for UX to decide.
@
13 years ago
This is what I came up with: Only display skip crop button, wenn flex-width or height are supported
#9
@
13 years ago
- Keywords has-patch added; reporter-feedback removed
- Milestone changed from Awaiting Review to 3.5
#10
@
13 years ago
Nacin just added on IRC, that the patch should also block the skip-cropping block of code in step_3().
I think MadtownLems is on it.
#11
@
13 years ago
I wasn't sure exactly the best way to deal with the crop code in step_3, but seeing as if we hid the button from the UI in step 2, the only way I could fathom that $_POSTskip-cropping? could be !empty() is if they were, in fact, cheatin', so I went that route.
#12
@
12 years ago
- Type changed from enhancement to defect (bug)
custom-header-21100-madtownlems.diff appears proper. In that regard, this is a bug fix.
I may be misunderstanding the screen grab you linked to, but it looks like your theme isn't respecting flexible height header image... if it was, it would have printed the entire image, not chopped it with a menu at 190px. Main intent of flex headers was to stop forcing users to use the theme's desired header dimensions. I wouldn't want us to remove that flexibility (in a network admin situation where you want to have complete control over what your site admins do, I think a plugin providing that option might be worth it?). Can you confirm if your theme was supporting the flex headers feature?