Reportback modal cropping#3673
Conversation
There was a problem hiding this comment.
Is there a reason why this is required here and not up top? Isn't it all getting compiled together anyhoo, so might as well just require it at top with everything else... unless I'm missing something?
There was a problem hiding this comment.
@weerd I figured we only needed it if cropping was enabled. Since this function only gets called when cropping is enabled, i put it here.
There was a problem hiding this comment.
Cool. I think that makes sense. I saw you removed it from at top of the app.js file. I figured since this all gets combined into a minified file it gets added regardless and probably more readable at top. Although I get the require stuff confused at times, so maybe @DFurnes can shed some light, haha. Fine to merge for now with this cause I don't know what the answer is :)
There was a problem hiding this comment.
@weerd ok cool. Yea I like require because it allows you to load js when you actually need it instead of all the time. I could even possibly require the js and then assign a callback function that runs the code that uses it only after the file has actually been added to the page. This way you don't add more js then you ever need. Happy to hear other thoughts on this though!
There was a problem hiding this comment.
Let's add it as is and if we need to change later no biggie :) It was more a question than an actual request, haha!
|
@weerd updated class names! Let me know what you think. |
|
Awesomeness 👍 |
*Links up the cropping functionality into the image upload modal and populates the main reportback form with the cropping coordinates.
*Moved cropping interface styles to the crop modal sass.
@weerd