Skip to content
This repository was archived by the owner on Oct 29, 2020. It is now read-only.

Reportback modal cropping#3673

Merged
sbsmith86 merged 7 commits intoDoSomethingArchive:devfrom
sbsmith86:reportback-modal-cropping
Dec 22, 2014
Merged

Reportback modal cropping#3673
sbsmith86 merged 7 commits intoDoSomethingArchive:devfrom
sbsmith86:reportback-modal-cropping

Conversation

@sbsmith86
Copy link
Contributor

*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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

@sbsmith86
Copy link
Contributor Author

@weerd updated class names! Let me know what you think.

@weerd
Copy link
Contributor

weerd commented Dec 22, 2014

Awesomeness 👍

sbsmith86 added a commit that referenced this pull request Dec 22, 2014
@sbsmith86 sbsmith86 merged commit 125d82a into DoSomethingArchive:dev Dec 22, 2014
@sbsmith86 sbsmith86 deleted the reportback-modal-cropping branch March 6, 2015 15:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants