This patch adds a more intuitive UI to the new image crop effect anchor widget. This is based off of the paradigm used by Photoshop and many other popular image editing applications. Discussion started here. http://drupal.org/node/371374#comment-1776588
This patch depends on #371374: Add ImageCache UI Core
The widget works by using javascript to add a class of .with-widget to the anchor widget table. Then in the CSS the radio buttons in the table are hidden and the arrows-sprite is used as a background for the table.
Javascript is then used to allow the user to click a td element in the table which toggles the hidden radio button.
You will also need to download the attached arrows-sprite.png file and place it into the modules/image/images/ directory
Caveat: I am not a designer at all, so if anyone has a better idea for the arrows-sprite please post it. This patch will fail tests miserably without the above patch and the image attached.
Comment | File | Size | Author |
---|---|---|---|
#33 | 525716-33-image_crop_arrows.patch | 5.18 KB | eojthebrave |
#33 | image-crop-arrows.png | 1.88 KB | eojthebrave |
#33 | 525716-33-with_image.patch | 7.61 KB | eojthebrave |
#32 | 525716-32.patch | 4.9 KB | star-szr |
#27 | 525716-anchor-padding.patch | 303 bytes | franz |
Comments
Comment #1
drewish CreditAttribution: drewish commentedsubscribing.
Comment #2
quicksketchJust a note, I think we'd be better off with an set of arrow more like the attached image, because we'll need arrow not only going outward (like the original arrows) but inward also. Of course we could fix this by just flipping all the arrow directions and having an image twice as big, but there seems something pure about reusing the same image multiple times (maybe not though).
We'll have to decide if this "arrows going in" versus "arrows going out" is really something we need to consider at all. Since this makes the assumption that we know what size the image is to begin with before the cropping occurs. If we *do* know the size (such as when an image has been scaled to 40 height before the crop), then you get a situation where you might have arrows like this:
Where the width is being cropped but the height is not.
Comment #3
quicksketchRight, and the arrows.
Comment #4
eojthebraveNew version of the patch uses a single set of arrows and positions the sprite in each td as needed. Should be visually identical to what is shown in the screenshots in #1.
I'm not really sure that the arrows going in vs. out is really necessary, could you point me to an example of this in real world usage? Does Photoshop or any similar application do anything like this?
Attached cropping-arrows.png needs to be placed in modules/image/images/
Comment #5
quicksketchI think it'd make sense to adjust the images via PHP and JavaScript adjusting classes (such as arrow-nw) rather than making CSS rules that match on 4+ different classes.
CSS such as:
Seems particularly excessive. Is there anything wrong with this?
As for arrow direction, yes Photoshop does this, which is where the crop-grid idea originated. I think it's fine if we decide to only use one direction for arrows, but it should probably be inward rather than outward, since the cropping operation will almost always size-down than image.
Comment #6
Bojhan CreditAttribution: Bojhan commentedThe selected box is not clear. I am not if I understand why we would need arrows going in?
Comment #7
quicksketchThe arrows are meant to indicate the effect cropping will have on the canvas. The arrows going in is meant to indicate "hey, the canvas is going to get smaller on these sides". In Photoshop if you adjust the canvas to be smaller, the arrows go in. If you adjust the canvas to be larger than the current size, the arrows go out.
Comment #8
eojthebraveAlright, here we go. Less CSS more Javascript. I am by no means a Javascript expert so if someone has a more elegant solution to this please chime in. PHP changes are still the same.
You'll need to download the image from #4 and place it in modules/image/images/ for this to work correctly.
Comment #9
eojthebrave@bojhan, suggestions for making the selected box better? Maybe a different color? Maybe an image like a small circle or something. I'm not coming up with anything that really jumps out at me.
Comment #10
eojthebraveChanges the background color of he selected position to the blue color used in the new admin theme. See attached images.
Requires arrows from #4 be downloaded and placed in modules/image/images/ and renamed to cropping-arrows.png
Comment #11
Bojhan CreditAttribution: Bojhan commentedNot the perfect colour, but this should be better.
Comment #12
drewish CreditAttribution: drewish commentedIn Safari the crop anchor isn't reloaded correctly when editing a crop. I did some clean up on the JS code to hopefully make it a bit easier to understand.
Comment #13
drewish CreditAttribution: drewish commentedi should make it clear that i fixed the issue where the anchor selection wasn't be saved.
Comment #14
eojthebraveThis looks good to me. Note that you'll need to download the cropping-arrows.png from #4, rename the file to cropping-arrows.png and place it in modules/image/images/cropping-arrows.png
Comment #15
mcrittenden CreditAttribution: mcrittenden commentedDoes anybody else find this less intuitive than the radio buttons we have now? Not that the radio buttons are a good solution (because they're definitely not), but these arrows are just confusing to me. The fact that they point towards the part of the image that will be cut off sort of looks like they're stretching in some way towards that direction, not that it's getting cut off. But if I'm the only one then feel free to ignore.
Comment #16
eojthebrave@mcrittenden I think that this is fairly typical of UI design for image manipulation programs. Almost every application that I can think of uses some similar widget for selecting an anchor when cropping/resizing the canvas or the image. It will at least be very intuitive for anyone that uses any desktop image manipulation tools.
Comment #17
mcrittenden CreditAttribution: mcrittenden commented@eojthebrave: that works for me...I guess my GIMP-only click-and-drag experience threw me off :) Thanks for the examples.
Comment #18
joshmillerThat crop icon is confusing because in reality, the arrows pointing out are the result of an image that is not changing size or getting bigger. Attached are screenshots of photoshop and what the arrows do when cropping.
Comment #19
drewish CreditAttribution: drewish commentedI think we might need to have double arrows <-> because we don't really know if we'll be expanding or contracting. We're cropping against *all* images so they could be larger or smaller in both dimensions.
Comment #20
joshmillerIf that is the case, then I suggest we simply indicate where the anchor point will be, attached is from Adobe Illustrator (same can be seen in InDesign, Photoshop, Flash, etc...)
This simplifies the icon considerably. The only problem becomes explaining this piece of information.
Comment #21
drewish CreditAttribution: drewish commentedjosh, I'm not really sure that's an improvement... might as well just leave it radio buttons in that case. i think the arrows are much more helpful.
Comment #22
eojthebraveOkay, here's an updated version that works again, and has arrows that point in both directions. Just noticed that the current styling for the anchor widget is messed up in both FF and Safari. See attached screenshot.
It would be nice to get this in, but if nothing else we should at least fix what we've got currently.
Comment #23
mcrittenden CreditAttribution: mcrittenden commented@#22, If we're forced to just fix what we have, the one line patch from #553938: Tweak CSS for image anchor table will do that, although it will probably need a re-roll. I'm attaching it here just to keep everything in one place.
Marking CNR for discussion. I'd LOVE to get #22 in. I tested it, it works well, is a pretty significant UX improvement IMO, and I couldn't find any problems with the code.
Comment #24
MichaelCole CreditAttribution: MichaelCole commented#23: anchor.patch queued for re-testing.
Comment #25
franz#23: anchor.patch queued for re-testing.
Comment #27
franzre-rolled latest patch. However, I'd really love to see the UI change in #22
Comment #28
Bojhan CreditAttribution: Bojhan commentedNeeds image/screencast for review.
Comment #29
franzBojhan, there is one on #22
Comment #30
Bojhan CreditAttribution: Bojhan commentedWhoa, ok - then its really confusing :)
Comment #31
xjmI take #30 to mean #22 is a definite improvement, and I'd agree. However, do we have a non-graphical fallback for this? We'd want invisible labels like "Upper right," etc., or maybe some text that appears on hover.
Can we get a reroll of #22 (and check for the element labels)? Not sure if that is backportable, but we could use #27 as the D7 backport if needed. (We might also want to backport the label fixes.)
Tagging as novice for the task of rerolling the Drupal 8.x patch.
Comment #32
star-szrRerolled #22 for 8.x and #1217006: Clean up the CSS for Image module.
Two changes/notes:
.image-anchor
selectors were refactored a bit in #1217006: Clean up the CSS for Image module. I mostly went with HEAD except for addingtable
to the initial.image-anchor
selector due to the markup changes introduced in this patch.I did a bit of manual testing and the patch works as expected.
One hiccup - we're missing the two-way arrow graphic! I was able to test using the one-way arrow icon PNG in #4 but I don't think the two-way arrows shown in #22 ever got uploaded to this issue. So that graphic may need to be recreated unless @eojthebrave can upload it here.
The arrows in question:
Comment #33
eojthebraveI had to dig a bit to find this but I did! Sweet.
I also renamed the file to image-crop-arrows.png and moved it to core/misc/ as that seemed more appropriate.
You need to either download the attached .png and place it in core/misc/image-crop-arrows.png or use the 525716-33-with_image.patch version of the patch which was created using `format-patch` and contains the image as well.
Finally, this seems like a good candidate for #d8ux so adding that tag.
Comment #36
mgiffordWhat happened to image_crop_form()? I can't seem to grep it.
Comment #39
marvil07 CreditAttribution: marvil07 as a volunteer commentedPatch does not apply, moving to NW.