Closed
Bug 895252
Opened 11 years ago
Closed 11 years ago
Implement crop
Categories
(Webmaker Graveyard :: Popcorn Maker, defect)
Webmaker Graveyard
Popcorn Maker
Tracking
(Not tracked)
VERIFIED
FIXED
People
(Reporter: brett, Assigned: thecount)
References
Details
(Whiteboard: popbetter)
Attachments
(1 file)
Often with images you'll want them to fill the maximum area allowed by the aspect ratio. Currently this is tedious - you need to move and scale the image to do this. An option to fill the canvas would speed things up for makers
Reporter | ||
Comment 1•11 years ago
|
||
Kate had demos in her drag and drop that solved this. Scott, can you find?
Flags: needinfo?(scott)
Summary: Image plugin needs option to fill screen → Implement crop
Assignee | ||
Comment 2•11 years ago
|
||
http://k88hudson.github.io/bestdraggable/#images-tab-container That's it. We should do that. We can also do a simple full screen button to the most common usecase of image resize even easier.
Flags: needinfo?(scott)
Reporter | ||
Comment 3•11 years ago
|
||
yes, do!
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → scott
Assignee | ||
Comment 4•11 years ago
|
||
Finally! Been tracking down some growth issues when I converted from using pixels to percentages. Took a bit to get right.
Attachment #797655 -
Flags: review?(schranz.m)
Comment 5•11 years ago
|
||
Comment on attachment 797655 [details] [review] https://github.com/mozilla/popcorn.webmaker.org/pull/189 Some questions for now. There is a lot of thought process behind this code that is hard to pick up by just reading it.
Attachment #797655 -
Flags: review?(schranz.m) → review-
Assignee | ||
Comment 6•11 years ago
|
||
Comment on attachment 797655 [details] [review] https://github.com/mozilla/popcorn.webmaker.org/pull/189 Updates.
Attachment #797655 -
Flags: review- → review?(schranz.m)
Comment 7•11 years ago
|
||
Comment on attachment 797655 [details] [review] https://github.com/mozilla/popcorn.webmaker.org/pull/189 I feel the code here is really solid, but would like a follow up ticket filed. Right now resizing the actual container on the top/left start's moving the actual image around inside it when really it shouldn't do that. There were technical reasons behind why it's doing it right now and i'd rather land the feature itself. There are however bugs. For example, after double clicking on the container after switching to the flickr slide show... https://dl-web.dropbox.com/spa/qxpeeqhhsjhcqcf/8t26yt5f.png
Attachment #797655 -
Flags: review?(schranz.m) → review-
Comment 8•11 years ago
|
||
Also, that tooltip shouldn't really be present when it's not a normal image. Should be able to use https://github.com/mozilla/popcorn.webmaker.org/commit/b3fcc5d1c11ccbf37d00dc7c0d49210d6bf29af5 to accomplish hiding it.
Comment 9•11 years ago
|
||
Probably going to want to add https://github.com/mozilla/popcorn.webmaker.org/blob/master/public/templates/assets/editors/editorhelper.js#L249-L251 and https://github.com/mozilla/popcorn.webmaker.org/blob/master/public/templates/assets/editors/editorhelper.js#L132-L134 to your code as well to prevent extra handles being generated.
Assignee | ||
Comment 10•11 years ago
|
||
Comment on attachment 797655 [details] [review] https://github.com/mozilla/popcorn.webmaker.org/pull/189 Updates.
Attachment #797655 -
Flags: review- → review?(schranz.m)
Comment 11•11 years ago
|
||
Comment on attachment 797655 [details] [review] https://github.com/mozilla/popcorn.webmaker.org/pull/189 Flickr Slideshow is broken. This is thrown to the console when I select it: GET http://localhost:8888/undefined 404 (Not Found)
Attachment #797655 -
Flags: review?(schranz.m) → review-
Assignee | ||
Comment 12•11 years ago
|
||
Comment on attachment 797655 [details] [review] https://github.com/mozilla/popcorn.webmaker.org/pull/189 Was there anything else? Anyway, flickr is doing this on master. Looks like a broken link in the first of three images under the mozilla tag. If you change the tag, it works fine, or if you play anyway, 2 of the 3 mozilla tagged images do load. First one is the dead link. I think we should file it.
Attachment #797655 -
Flags: review- → review?(schranz.m)
Comment 13•11 years ago
|
||
Comment on attachment 797655 [details] [review] https://github.com/mozilla/popcorn.webmaker.org/pull/189 R+ with nits in comments and: public/templates/assets/editors/image/image.js: line 30, col 5, Function declarations should not be placed in blocks. Use a function expression or move the statement to the top of the outer function. public/templates/assets/editors/image/image.js: line 42, col 11, '$' is not defined. public/templates/assets/editors/image/image.js: line 43, col 7, '$' is not defined. public/templates/assets/editors/image/image.js: line 141, col 7, '$' is not defined. public/templates/assets/editors/image/image.js: line 158, col 7, '$' is not defined. These are lint errors introduced by this patch. I know it isn't passing lint right now but if we don't introduce more that would be nice. Also, if you haven't filed a bug for that problem with flickr, can you?
Attachment #797655 -
Flags: review?(schranz.m) → review+
Assignee | ||
Comment 14•11 years ago
|
||
Comment on attachment 797655 [details] [review] https://github.com/mozilla/popcorn.webmaker.org/pull/189 Filed it as bug 915738 I fixed the lint issues and added a comment. Going to throw it back to you for one last sanity check.
Attachment #797655 -
Flags: review+ → review?(schranz.m)
Updated•11 years ago
|
Attachment #797655 -
Flags: review?(schranz.m) → review+
Assignee | ||
Comment 15•11 years ago
|
||
Staged: https://github.com/mozilla/popcorn.webmaker.org/commit/c149b3991a801322c7dc02d32300590476bb7695 Needs verification.
Status: NEW → RESOLVED
Closed: 11 years ago
Flags: needinfo?(scott)
Resolution: --- → FIXED
Assignee | ||
Updated•11 years ago
|
Status: RESOLVED → VERIFIED
Flags: needinfo?(scott)
Updated•11 years ago
|
Attachment mime type: text/plain → text/x-github-pull-request
You need to log in
before you can comment on or make changes to this bug.
Description
•