Problem/Motivation
The label and help text for the maximum and minimum resolution settings on image fields could be improved.
Current text
Maximum image resolution
The maximum allowed image size expressed as WIDTH×HEIGHT (e.g. 640×480). Leave blank for no restriction. If a larger image is uploaded, it will be resized to reflect the given width and height. Resizing images on upload will cause the loss of EXIF data in the image.
Minimum image resolution
The minimum allowed image size expressed as WIDTH×HEIGHT (e.g. 640×480). Leave blank for no restriction. If a smaller image is uploaded, it will be rejected.
Steps to reproduce
- Install Drupal
- Edit the image field on Article content type, or add a new image field to a content type
- See the help text for 'Maximum image resolution' and 'Minimum image resolution'
Proposed resolution
Update the help text to:
Maximum image dimensions
The maximum allowed image dimensions as WIDTH×HEIGHT (e.g. 640×480). Leave blank for no restriction. If a larger image is uploaded, it will be resized to the given width and height. Depending on the toolkit in use, resizing images on upload may cause the loss of image metadata.
Minimum image dimensions
The minimum allowed image dimensions as WIDTH×HEIGHT (e.g. 640×480). Leave blank for no restriction. If a smaller image is uploaded, it will be rejected.
Remaining tasks
- Agree on the wording change
- Write a patch
- Review
User interface changes
Before patch:
After patch:
TBC
API changes
N/A
Data model changes
N/A
Release notes snippet
N/A
Comment | File | Size | Author |
---|---|---|---|
#30 | afterpatch.png | 57.5 KB | Rinku Jacob 13 |
#30 | beforepatch.png | 54.65 KB | Rinku Jacob 13 |
#29 | 847098-29.patch | 2.6 KB | ranjith_kumar_k_u |
#18 | after.png | 13.52 KB | mondrake |
#18 | before.png | 12.41 KB | mondrake |
Comments
Comment #1
danyalejandro CreditAttribution: danyalejandro commentedConfirmed. The minimum image resolution description needs work as well.. Fixing this would take like 10 mins IMO.
Comment #2
glipay CreditAttribution: glipay commentedI changed the maximum image resolution description to read "The maximum allowed image size expressed as width followed by height (e.g. 640 480 for a maximum with of 640 pixels and a maximum height of 480 pixels). Leave blank for no restriction. If a larger image is uploaded, it will be resized to reflect the given width and height. Resizing images on upload will cause the loss of EXIF data in the image." I changed the minimum image resolution descripiton to read "The minimum allowed image size expressed as width followed by height (e.g. 640 480 for a minimum width of 640 pixels and a minimum height of 480 pixels). Leave blank for no restriction. If a larger image is uploaded, it will be rejected."
Comment #3
joachim CreditAttribution: joachim commentedWe don't need to say 'expressed as' any more. That was a bit of verbiage to mean 'you're going to have to muck around in this text field'.
We can just say 'the maximum width and height of an image'.
Comment #4
glipay CreditAttribution: glipay commentedhere's a new patch then using the wording you sugested.
Comment #5
tstoecklerThat should be "if a SMALLER file is uploaded" for minimum dimensions
Comment #6
glipay CreditAttribution: glipay commentedoops, I fixed the problem now
Comment #7
joachim CreditAttribution: joachim commentedDoes this need to go on 8.x first?
Comment #8
tstoecklerYes, that is correct.
Comment #9
tstoecklerAlso marking "needs work", because patch needs to be rerolled. It *might* apply by simply patching from within the /core directory, but I didn't try that.
Comment #10
RobW CreditAttribution: RobW commentedThere's still an 'x' in between the width and height fields, so 640 x 480 is a better description than 640 480. But maybe we should take a step back and revise the whole field for better usability.
There are suggestions in this (ancient) issue #1215730: Terminology update: don't say "Resolution" when we mean "Dimensions" that are applicable here. Dimensions and resolution mean different things.
I propose:
Maximum Image Dimensions
|_____|px width |_____|px height
If a larger image is submitted it will be resized to these dimensions, which may erase existing EXIF data. Leave blank for no restrictions.
Minimum Image Dimensions
|_____|px width |_____|px height
If a smaller image is submitted it will be rejected. Leave blank for no restrictions.
Text Changes:
Code Changes:
_image_field_resolution_validate
to_image_field_dimensions_validate
.Also, #1079116: Inaccurate text: Images must be smaller than !max pixels. is addressing similar issues, but for the node/entity edit form, not the admin UI. Noted here for reference.
Comment #11
RobW CreditAttribution: RobW commented"existing Exif data" could probably be replaced with the more readable and correct "image metadata". People who care will know what it's talking about. I am assuming that the resize also strips Photoshop metadata, which contains but isn't limited to Exif, although I haven't tested.
Comment #12
mgiffordComment #13
snehi CreditAttribution: snehi as a volunteer and at Publicis Sapient for Publicis Sapient commentedComment #14
snehi CreditAttribution: snehi as a volunteer and at Publicis Sapient for Publicis Sapient commentedComment #15
alvar0hurtad0This is the reroll agains 8.1.x branch
Comment #17
mondrakeThis new patch addresses only the UI aspects along the lines of what @RobW suggested in #10 and #11, for parts that were not addressed separately already.
Comment #18
mondrakeScreenshots:
Before:
After:
Comment #29
ranjith_kumar_k_u CreditAttribution: ranjith_kumar_k_u at Zyxware Technologies commentedRe-rolled the last patch for 9.3
Comment #30
Rinku Jacob 13 CreditAttribution: Rinku Jacob 13 at Zyxware Technologies commentedVerified and tested patch#29 for drupal 9.3.x-dev version. Patch applied successfully and looks good to me.Adding screenshot for the reference.
Need +1 RTBC
Comment #31
pameeela CreditAttribution: pameeela commentedI've updated the issue summary to reflect the current state but also to tweak the wording as I don't really think the current patch is that much of an improvement. I've taken the suggestion in #11 to simplify it by removing the EXIF reference. I think "image metadata" is clear enough.
But I'm far from an expert in this so could use someone who knows to review the proposed change.
Comment #32
pameeela CreditAttribution: pameeela commentedFixed weird image sizing/spacing.