Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support resize of viewport #347

Merged
merged 9 commits into from
Jul 14, 2017
Merged

Conversation

fdanielsen
Copy link
Contributor

This adds a brand new feature supporting resize of the viewport. It's an option that needs to be explicitly turned on, and one can choose whether height and/or width should be possible to change. We've used this feature ourself in production for a while, mostly locking resize to height, and it works well as far as we've seen. Although support on touch devices have not been tested.

Since an image is usually in the document flow, we feel it makes most sense to only allow dragging to resize on the bottom and right side of the image. This adds handles, similar to crop+resize in Photoshop and other image editors, at the bottom and right of the viewport. Clicking and dragging on the handles will update the viewport size dynamically. Size is limited to a minimum of 50px, and a maximum of the current height of the actual image.

I've added a demo of it on the official demo page. Since tests does not seem to be used much/up to date, I haven't added any new tests for this.

Any input is very welcome, and I hope we can make this fit in the main version of Croppie making it easier for us to keep our use up to date.

An option to enable resizing has to be set to `true`, while further
customization of which axis can be resized can also be configured.

With resizing enabled a resize handle at the bottom and/or the right of
the viewport will be displayed. Hovering over it will change the cursor
to indicate resizability. Dragging this handle will update the viewport
and boundary to the new size interactively, but the axis will be
constrained to the end of the image.
We previously bounded the maximum height or width one could resize the
image up to according to how much of the image was left below or to the
right of the viewport. But after integrating more deeply with croppie's
other updates we don't need to do that. We can simply limit it to the
actual image height or width and croppie will ensure to center the image
appropriately.
@thedustinsmith
Copy link
Contributor

Thanks for the PR - there's been a lot of interest in this one. I'm motivated to get it in the master branch, I just need to find time. Possibly later this week.

@fdanielsen
Copy link
Contributor Author

Great! Yes, I think I commented on an earlier thread about this and said we'd be interested in helping out getting this in place. Sorry it's taken so long.

I conveniently pushed out this PR the day before going on a weeks holiday. But if you get the time to look more closely at it this week I'll be ready to make any changes starting Tuesday next week.

@thedustinsmith
Copy link
Contributor

This is just awesome, and it works surprisingly well. I wasn't sure how well a resizable viewport would work within croppie, but I'm impressed.

I'm not interested in holding this one up for feature creep, but I can see adding some additional options to this.

  • It'd be nice to have the handles on the top and bottom, left and right
  • I'd like the ability to not resize the boundary when I'm resizing the viewport. I'm guessing there's a reason you did it for your use case so I don't think we should change it immediately, but I think it could be a bit more useful in general use cases.

That's really it. I'm going to go ahead and get this one merged into master. Super exciting.

If you'd like to work on the other options, that'd be great, but if not, no big deal. Thanks a ton for helping out.

@thedustinsmith thedustinsmith merged commit 69d5515 into Foliotek:master Jul 14, 2017
@fdanielsen
Copy link
Contributor Author

Awesome, glad you like the feature. Regarding your two feature suggestions, I don't think we have a particular need for those so I'm not sure how soon I'd be able to look into implementing that. But I have a couple questions about them to clarify the needs:

  • One of the reasons we didn't implement handles on the top and left is that our images are positioned in the document flow, as opposed to absolutely positioned as layers on top. That means even if you drag upwards or to the left, the image would still stay at the same top/left position and the opposite side would extend while you drag. Sure, the editor itself is absolutely positioned, but I think what you see in the editor (including position) should be the same as when you exit. So that doesn't sound very intuitive or pleasant to me. But maybe you have some thoughts on how that would behave?
  • Hm, so by not resizing the boundary, the boundary acts as a maximum size? Sounds fair to me as an option. We see the boundary more as an indication that you're in edit mode, where you can see where the image extends outside the crop. If that makes sense to you.

Anyway, thanks for merging this into master. And thanks for making a new release. We will make use of this new version in our product this week, and see if we come across any issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants