Fix crop not working. Inverted logic. Width always comes first!

humanmade / WPThumb

:warning: UNMAINTAINED :warning: On demand image resizing for WordPress

https://humanmade.co.uk/wpthumb/

170 stars 32 forks source link

Fix crop not working. Inverted logic. Width always comes first! #65

Closed mattheu closed 11 years ago

mattheu commented 11 years ago

Crop to position wasn't working. Discovered it was getting x and y axis mixed up. Not sure how long it's been like this.

Note that this failed unit tests.

mattheu commented 11 years ago

Hmm...

The unit tests now fails because the test specifies crop position as bottom,right The crop to position meta value is saved as right,bottom

One is wrong! My vote is with the unit test.

As width always comes before height it seems to make sense for horizontal crop position to come before vertical.

I don't think it is right to handle both as we could technically be specifying crop position with numeric values

joehoyle commented 11 years ago

I am not so sure it would be horizontal, vertical. "top, left" sounds a lot more natural than "left, top", "botom, right" etc. Looking at https://github.com/humanmade/WPThumb/wiki/Watermarking (which is wrong as we then used "tl" for top, left, however it follows "vertical, horizontal" so we should probably stick with that.