-
Notifications
You must be signed in to change notification settings - Fork 22
Reportback modal cropping #3673
Changes from 6 commits
1d2fac4
64855ca
ba048ac
9691afb
9efb6a3
888faab
8b3d134
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -55,4 +55,52 @@ | |
| max-width: 100%; | ||
| } | ||
| } | ||
|
|
||
| // Styles for cropping. | ||
| .image-crop-container { | ||
| position: relative; | ||
| display: inline-block; | ||
|
|
||
| .crop-box { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not 100% sure but could you do the whole
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @weerd sure i will try renaming some stuff. For this one it is interesting because it is actually acting like a container since the main purpose of this div is to create a bounding box for cropping. Maybe I can make it
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| position: absolute; | ||
| z-index: 1; | ||
| top: 10px; | ||
| left: 10px; | ||
| border: 1px dashed $blue; | ||
| background-color: rgba(255, 255, 255, 0.3); | ||
|
|
||
| &:hover { | ||
| cursor: move; | ||
| } | ||
| } | ||
|
|
||
| .resize-handle-wrapper { | ||
| position: relative; | ||
| width: 100%; | ||
| height: 100%; | ||
| } | ||
|
|
||
| .resize-handle { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also here with the |
||
| position: absolute; | ||
| right: 0; | ||
| bottom: 0; | ||
| width: 40px; | ||
| height: 40px; | ||
|
|
||
| &:hover { | ||
| cursor: se-resize; | ||
| } | ||
| } | ||
|
|
||
| .resize-triangle { | ||
| width: 0; | ||
| height: 0; | ||
| top: 35px; | ||
| left: 35px; | ||
| position: absolute; | ||
| background: $blue; | ||
| width: 10px; | ||
| height: 10px; | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 :)
There was a problem hiding this comment.
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!
There was a problem hiding this comment.
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!