Handle EXIF metadata #156
Merged
Handle EXIF metadata #156
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jywarren
reviewed
Feb 27, 2019
| } | ||
| }), | ||
|
|
||
| EnableEXIF = EditOverlayAction.extend({ |
There was a problem hiding this comment.
Ooh wow!!!
Do you think we should start moving these into separate files, though, to keep the length of this file a bit lower?
This is great work!!!
There was a problem hiding this comment.
@jywarren I will mark this draft PR for review after some refactoring changes (including separating files) as I mentioned in the initial commit above. I had this (and some other changes) in mind but since I was working on multiple PRs back then so I just drafted this one here and tended to the others meanwhile. Sorry for any inconvenience/confusion caused! I'll push the changes and mark this for review very soon!
|
no problem and thanks a lot!!! Also note I posted an issue for breaking up
tools into separate files which could help simplify our development of new
tools like this.
…On Thu, Feb 28, 2019 at 9:17 PM Pranshu Srivastava ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/edit/DistortableImage.EditToolbar.js
<#156 (comment)>
:
> addHooks: function ()
{
var editing = this._overlay.editing;
editing._toggleExport();
- this.disable();
+ this.disable();
+ }
+ }),
+
+ EnableEXIF = EditOverlayAction.extend({
@jywarren <https://github.com/jywarren> I will mark this draft PR for
review after some refactoring changes (including separating files) as I
mentioned in the initial commit above. I had this (and some other changes)
in mind but since I was working on multiple PRs back then so I just drafted
this one here and tended to the others meanwhile. Sorry for any
inconvenience/confusion caused! I'll push the changes very soon!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#156 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABfJ1ncAxhaLcPCxHVjOb0kIfSoe0gHks5vSI22gaJpZM4bQIdp>
.
|
|
@jywarren Please review. |
jywarren
approved these changes
Mar 5, 2019
examples/index.html
Outdated
| @@ -17,6 +17,9 @@ | |||
| <script src="../node_modules/webgl-distort/dist/webgl-distort.js"></script> | |||
| <script src="../node_modules/glfx/glfx.js"></script> | |||
|
|
|||
| <!-- for EXIF geocode --> | |||
| <script type="text/javascript" src="../node_modules/exif-js/exif.js"></script> | |||
There was a problem hiding this comment.
Do you prefer this to compiling it in? Thanks!
There was a problem hiding this comment.
@jywarren Should I add a browserify/requireJS support to this lib for the same?
|
Ah, though it'll need a rebase! Thank you @rexagod, this is super! |
|
🎉🎉🎉 |
|
Sure, but in this lib don't we currently concat only and aren't using full
browserify (although perhaps we should?)? Thanks!
…On Thu, Mar 14, 2019 at 12:48 PM Pranshu Srivastava < ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In examples/index.html
<#156 (comment)>
:
> @@ -17,6 +17,9 @@
<script src="../node_modules/webgl-distort/dist/webgl-distort.js"></script>
<script src="../node_modules/glfx/glfx.js"></script>
+ <!-- for EXIF geocode -->
+ <script type="text/javascript" src="../node_modules/exif-js/exif.js"></script>
@jywarren <https://github.com/jywarren> Should I add a
browserify/requireJS support to this lib for the same?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#156 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABfJx3Noh2kGyTItbsL65UNCnDBZJIlks5vWn0fgaJpZM4bQIdp>
.
|
Open
10 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #142.