Problem/Motivation
During the usability study, many participants were confused by the image width and height inputs when embedding an image through CKeditor. They thought they needed to supply this information.
I would think that the width and height should be automatically filled when I upload the image
The height/width inputs output directly to the width and height attributes of the image tag. This don't resize or crop the image, it just sets a visual size. It's also problematic for responsive themes, as the CSS styling usually overrides this setting so it can display correctly on different screen sizes. At best these values lie to the user, as a theme can override them. At worst, they allow content creator to break the look and feel of the site.
Proposed resolution
Remove the fields from the settings form. They are outdated and rarely used. Lewis and Marc Drummond spoke about this at the TCDC sprints, and he agreed it was more problematic to leave them in. He also suggested we add an image style selector in 8.1.
Remaining tasks
Agree to remove these inputs
Patch
Test
User interface changes
Removing the width and height form inputs for CKeditor image embeds
API changes
None
Data model changes
None
Comment | File | Size | Author |
---|---|---|---|
#12 | 2519862-remove-width-height-12.png | 193.59 KB | kattekrab |
#12 | 2519862-dialog-12.png | 22.17 KB | kattekrab |
#4 | Screen Shot 2015-08-13 at 14.04.13.jpg | 381.8 KB | LewisNyman |
#3 | remove_image_dimensions-2519862-3.patch | 1.72 KB | mErilainen |
Comments
Comment #1
DuaelFrI had the same questions from a lot of users.
They often don't understand at all what's the difference between resizing the picture offline to optimize it's size and resizing the picture using these inputs. They are also confused by the fact that Drupal resizes images in dedicated fields but not in the editor.
It would be really fantastic to be able to select an image style (or even a responsive image style) in this dialog.
Comment #2
tkoleary CreditAttribution: tkoleary at Acquia commented@DuaelFr
You took the words right out of my mouth. Wim Leers and I have actually already discussed something similar to this. The problem, I *think*, is that the image styles can only be applied to image fields. That can be done in D8 by installing entity embed module but that puts this squarely in contrib for the forseeable future.
Comment #3
mErilainen CreditAttribution: mErilainen at Wunder commentedThis patch will simply remove the fieldset and the dimension fields from the image upload dialog.
Comment #4
LewisNymanGreat, this works as expected, assuming the tests pass. As we're removing functionality I'm assigning to Wim for the final review.
Comment #5
Wim LeersI'm personally completely in favor of this. And we're already working on the latter in #2061377: [drupalImage] Optionally apply image style to images uploaded in CKEditor 5. But, until we have that… what if a user uploads a multi-megapixel image? It'll then be enormous. In other words: we're expecting people to upload images of a sane size.
Are we sure that that really is a better trade-off?
Comment #6
DuaelFrIMHO, not allowing users to set the widht and height in the dialog is better because if they upload an huge image, they'll see it immediately while (pre)viewing their content. I think that's a lot better than letting them think the image has been properly scaled down - what's clearly not the case with the current behavior. We might keep in mind that most of the writers dont know anything about HTML and have no idea about how the
<img>
tag really works.Comment #7
Wim LeersThat makes a ton of sense!
You're absolutely right. Until we have #2061377: [drupalImage] Optionally apply image style to images uploaded in CKEditor 5, the behavior proposed in this patch makes more sense. You've convinced me :)
Considerations:
Therefore, this is ready. I manually tested it, and it works fine.
Comment #8
Wim LeersComment #9
tkoleary CreditAttribution: tkoleary commentedWhile we're at it maybe we can get rid of the align stuff too. ;)
Comment #10
Wim LeersLet's not broaden the scope and then get stuck.
Comment #11
kattekrab CreditAttribution: kattekrab at Creative Contingencies commentedHmmm I tend to agree with removing the width & height fields here.
Along with @DuaelFr's excellent point about it not actually scaling the image, it's too easy to get them wrong, and distort the image.
But I disagree with @tkoleary about removing the alignment field - I find that really really useful. Unless there's another, easier way to set images left, right or centre, then I don't see why we would consider removing that.
I'm firing up SimplyTest me for a manual review now.
Comment #12
kattekrab CreditAttribution: kattekrab at Creative Contingencies commentedYep. RTBC ++
That feels like a UI improvement to me.
Dialog is cleaner.
Still able to place an image, add a caption, align, etc. And resizing the image through the UI adds height and width in correct ratios.
Comment #13
DuaelFrDo we still want to allow in-place resizing using the UI? I'm not sure.
That leads to the same problem where users doesn't understand that their huge picture is still huge after being resized there.
I think we should totally disable that functionnality and let contrib reintroduce it if needed. In the _near_ future, we'll be able to select an image style right from this Dialog so it should not be necessary anymore to have the ability to resize the image again.
What do you think?
Comment #14
Wim Leers#13: IMO: one step at a time. This is the actual problem raised in usability tests. Let's fix this first.
Comment #15
DuaelFr@Wim Leers : agreed!
Comment #16
alexpottWe were obviously missing test coverage of this feature too :)
Committed 9e2450b and pushed to 8.0.x. Thanks!
Comment #18
Wim LeersIndeed :(
Wim--
See #2395377: Write integration test for EditorImageDialog for adding such test coverage.