backdrop-issues icon indicating copy to clipboard operation
backdrop-issues copied to clipboard

[UX] Add an option to "lock" aspect ratio when resizing images in the editor dialog

Open jenlampton opened this issue 8 years ago • 36 comments

When resizing images in CKEditor, 99% of the time the user will want the aspect ratio to remain the same. We should make an option to do this (and enable it by default) so that people have a better experience resizing images in Backdrop.

Note that resizing within the editor iframe already maintains aspect ratio, this is just about the aspect ratio within the image dialog.

jenlampton avatar Aug 11 '16 00:08 jenlampton

I think that perhaps this user need is now met with issue #3200.

Graham-72 avatar Sep 03 '18 21:09 Graham-72

How do we imagine this to look/work? Should the aspect ratio lock be a checkbox (which allows for a label), or a lockpad icon (which saves on screen real estate)?:

image

This is what the image dialog (WYSIWYG) looks like currently, after having uploaded a new image:

screen shot 2018-10-19 at 10 11 26 am

...and here's what the image field looks like after uploading an image:

screen shot 2018-10-19 at 10 12 06 am

Things to note (do not necessarily need to be solved as part of this ticket here):

  1. The image dialog does not have an image thumbnail.

  2. The image width/height fields are not populated with the image dimensions. These are populated when editing an already uploaded image though:

screen shot 2018-10-19 at 10 15 52 am

...is there a technical limitation, or intentional for some reason?

klonos avatar Oct 18 '18 23:10 klonos

@klonos these are excellent points you have raised here about the editor's image dialogue and go beyond the original title of this issue. Maybe it should become a new issue that is a review of the image dialogue as we have it now, including use of the image library. Or perhaps we should have a meta issue that is a subset of the CKEditor meta issue #1087 which I find too all-encompassing to get focus on steps we might take for further improvement.

One thing I would like to see added is an indication in the image library of the size of each image for cases where it has two or more versions of the same image.

Graham-72 avatar Oct 20 '18 05:10 Graham-72

@Graham-72 sure ...as I said, these do not necessarily need to be solved as part of this ticket here. What I usually do with almost all tickets once they are closed is to go through all past comments and file follow-ups for any ideas or points discussed that were not part of the final solution. It might take me some time, but I eventually get through all/most of them ...and I am happy to see that quite often somebody else has beaten me to it 😄

I honestly do not mean to derail discussions; just logging my thoughts and UI/UX reviews for future reference ...especially during the "needs feedback" status of tickets. I will file follow-up tickets; rest assured 😉

...as for the META, I think one is enough. The way I see metas work is that people can discuss freely as time goes by, and any ideas/plans/tasks eventually end up in the issue summary (which is usually kept up to date by people like myself that have edit permissions in the GitHub queue 😄).

klonos avatar Oct 20 '18 08:10 klonos

@klonos I'm glad you are there with your encyclopedic knowledge of all issues Backdrop, D7 and D8. Personally, with my less agile brain, I would like to see an issue with a title something like 'Improve WYSIWYG image dialogue', grouping together ideas that exist and arise, and one can focus on as a group. But in the absence of this I will keep my own list in a personal memo. 😃

Graham-72 avatar Oct 21 '18 16:10 Graham-72

I just had a specific request for this "Lock proportions" concept in the WYSIWYG editor popup, so +1 from me on this idea. I like the little lock icon and I think that's pretty clear (as long as it fits with Backdrop's UI guidelines -- do we do anything like that elsewhere?).

laryn avatar May 08 '20 20:05 laryn

I guess, the "needs more feedback" label can get removed. (Another plus one from me.)

indigoxela avatar Sep 22 '20 11:09 indigoxela

Crazy idea: what if we just drop these two number inputs? The editor still provides the "magic corner" to resize the image via mouse drag.

In their current state, they're not very helpful, but can also cause problems (results in squeezed images - depends on the theme).

Reading the comment here after quite some time... Hey when would anyone ever want an image to lose its proper proportions - is there a weird use case for squeezed images I'm not aware of? :laughing:

So dropping the form items is one option, but properly keeping the numbers in sync might be the more bc way.

indigoxela avatar Oct 04 '22 15:10 indigoxela

Crazy idea: what if we just drop these two number inputs? The editor still provides the "magic corner" to resize the image via mouse drag.

Not crazy idea, but does not take into consideration people that cannot use a mouse (either because of disability, or because of hardware restrictions)

...when would anyone ever want an image to lose its proper proportions - is there a weird use case for squeezed images I'm not aware of? 😆

I second that. Once we introduce the aspect ratio lock, I believe that locked should be the default.

klonos avatar Oct 04 '22 19:10 klonos

Not crazy idea, but does not take into consideration people that cannot use a mouse

@klonos you're of course right.

Here's a PR for testing and review. This is, how far I got with my js skills. :wink:

Maybe a "reset to original" button would be a good thing - but I failed to get that working properly. And I also wouldn't know where to put the styles.

indigoxela avatar Oct 06 '22 12:10 indigoxela

Thanks for the PR! I've had a quick look at the sandbox. Works quite good, but I noticed one issue: When I change one of the dimensions (width or height) directly after uploading the image, the input of the other dimension gets empty, so there is no longer a value in that input. (When I insert the image and edit it again, both values are present.)

Tested in Chrome on Mac.

olafgrabienski avatar Oct 07 '22 10:10 olafgrabienski

When I change one of the dimensions (width or height) directly after uploading the image, the input of the other dimension gets empty

This may be a matter of msecs - can't reproduce with Chrome on Linux. But I know, that there's a minimal delay as js needs to read the uploaded image.

@olafgrabienski are you able to reproduce reliably, or does this only happen once in a while? Other browser? Other OS (if available)?

indigoxela avatar Oct 07 '22 11:10 indigoxela

Yes, I can reproduce it reliably, also with Firefox (but no other OS at hand).

In case I didn't explain the issue very well:

  • Upload an image, and have a look at the dimensions, e.g. 600 x 450 pixels.
  • Change the value of the first input e.g. to 599, by using arrow keys or by typing numbers. If the latter, leave the input again.
  • Look for the value of the second input. It is empty, and it stays empty (no question of milliseconds).

olafgrabienski avatar Oct 07 '22 11:10 olafgrabienski

@olafgrabienski anything in the console log of your browser dev tools?

indigoxela avatar Oct 07 '22 11:10 indigoxela

Oh yes, Console log directly after changing the first value:

The specified value "NaN" cannot be parsed, or is out of range.
(anonymous) @ js_jj9emzsJm-vHdoXG0tCcmcQC1XMJxAw06One83BLjEQ.js:4
each @ js_jj9emzsJm-vHdoXG0tCcmcQC1XMJxAw06One83BLjEQ.js:2
each @ js_jj9emzsJm-vHdoXG0tCcmcQC1XMJxAw06One83BLjEQ.js:2
val @ js_jj9emzsJm-vHdoXG0tCcmcQC1XMJxAw06One83BLjEQ.js:4
(anonymous) @ js_u_T1_VKlmsWeXowUntSEOaY4quYH_MBrSmEKEPbtfmg.js:1221
dispatch @ js_jj9emzsJm-vHdoXG0tCcmcQC1XMJxAw06One83BLjEQ.js:3
r.handle @ js_jj9emzsJm-vHdoXG0tCcmcQC1XMJxAw06One83BLjEQ.js:3

olafgrabienski avatar Oct 07 '22 11:10 olafgrabienski

The specified value "NaN" cannot be parsed, or is out of range.

Oops, it's not a number. :thinking: But which step fails...

We should turn off aggregation.

indigoxela avatar Oct 07 '22 11:10 indigoxela

With aggregation turned off I can reproduce it in the Tugboat sandbox (but not locally - my "locally" is remote).

Nothing in my console log... But also no number sync.

indigoxela avatar Oct 07 '22 11:10 indigoxela

In sandbox without aggregation:

The specified value "NaN" cannot be parsed, or is out of range.
(anonymous) @ jquery.js?v=1.12.4:4
each @ jquery.js?v=1.12.4:2
each @ jquery.js?v=1.12.4:2
val @ jquery.js?v=1.12.4:4
(anonymous) @ filter.js?v=1.24.x-dev:384
dispatch @ jquery.js?v=1.12.4:3
r.handle @ jquery.js?v=1.12.4:3

olafgrabienski avatar Oct 07 '22 11:10 olafgrabienski

I think I found a way to reproduce reliably - and it seems to be a logic problem. It's attempted to get the image dimensions before the "onload" of the image has succeeded. Have to think this over... :thinking:

More testing is still welcome!

indigoxela avatar Oct 07 '22 12:10 indigoxela

I think I found a way to deal with the asynchronous image loading. Not sure if it's the most elegant solution (but reading all that promise/resolve/then stuff again caused me headache :stuck_out_tongue_winking_eye:).

Further testing would be cool!

indigoxela avatar Oct 07 '22 14:10 indigoxela

Further testing would be cool

Great, works for me in the sandbox!

olafgrabienski avatar Oct 07 '22 14:10 olafgrabienski

I have a question about this. I'm not sure if I'm missing something, but I don't see the option to lock/unlock the aspect ratio. As far as I can tell, this PR effectively disables the ability to change the aspect ratio/proportions of an image. I admit this is unusual, but I have used images in that past where the aspect ratio was not essential (non-photos for example), and the ability of changing only the height but not the width came handy.

There is some conversation above adding a lock. Is that still being considered for this PR?

argiepiano avatar Oct 08 '22 17:10 argiepiano

@indigoxela thanks again. I left some comments.

argiepiano avatar Oct 08 '22 18:10 argiepiano

I have a question about this. I'm not sure if I'm missing something, but I don't see the option to lock/unlock the aspect ratio

I have been in the situation where I have an image that is already skewed in its original state and one way to fix that is to stretch one dimension to make it look passable. I think we need an option to unlock.

docwilmot avatar Oct 08 '22 18:10 docwilmot

I'm not sure if I'm missing something, but I don't see the option to lock/unlock the aspect ratio.

That's based on the previous discussion here - who would ever want a squeezed image?

I have been in the situation where I have an image that is already skewed in its original state and one way to fix that is to stretch one dimension to make it look passable. I think we need an option to unlock.

Bare with me, this may take some time. I'm working on the edges of my js skills already. Further tweaks and additional features might completely be out of reach for me. So if someone with better js skills wants to take over here - feel free to do so.

If we need buttons - where should the styles go (they will require styling)? To the filter module, to system css, or to the themes. I'm aware that this discussion will block progress here for a while.

indigoxela avatar Oct 09 '22 09:10 indigoxela

Let's see if we can revive this issue.

@docwilmot wants an option (button) to unlock - and that's something I don't want at all.

We could...

  • Provide an option in the filter admin settings to toggle the behavior (no lock+sync OR lock+sync), off for existing sites
  • I could provide this functionality as contrib module - so people have the choice

Would either of that bring us a step forward? Feedback is welcome!

BTW: looking at the current issue label - I don't think that "task" is still the right one, it's rather "feature". IMO the change isn't small enough for a task anymore.

indigoxela avatar Nov 15 '22 09:11 indigoxela

I could provide this functionality as contrib module

Are you speaking about the lock functionality or about the unlock option?

olafgrabienski avatar Nov 15 '22 09:11 olafgrabienski

Are you speaking about the lock functionality...

The lock functionality, of course. When it's in contrib, it's optional, anyway.

If in core, then with a newly added on/off setting per filter format via admin UI (not per image).

indigoxela avatar Nov 15 '22 10:11 indigoxela

The lock functionality, of course

Thanks for the feedback! I guess that wasn't obvious to me because the lock functionality was discussed as a 99% use case (while the unlock option was mentioned as rather unusual, even by people who asked for it). So, I'm still for putting the lock functionality in core. And yes, an unlock setting may be a good compromise.

olafgrabienski avatar Nov 15 '22 10:11 olafgrabienski

@docwilmot wants an option (button) to unlock - and that's something I don't want at all.

FWIW I also support the inclusion of the lock/unlock button. This is something that's standard in pretty much every image UI I've ever used (granted, I'm no expert). It seems strange that this is not included here, and the rationale for not including it seems flawed (b/c it's hard?)

D7 CKEditor: Screen Shot 2022-11-15 at 6 42 44 AM

argiepiano avatar Nov 15 '22 13:11 argiepiano