Fix drop image rotate wrong #2322
Conversation
Fix drag-drop image rotate wrong, as Jordan Thornquest mentioned in slack
| if (!fs.existsSync(sourceFilePath)) { | ||
| reject('source file does not exist') | ||
| const isBase64 = typeof sourceFilePath === 'object' && sourceFilePath.type === 'base64' | ||
| if (!fs.existsSync(sourceFilePath) && !isBase64) { |
There was a problem hiding this comment.
Why do you reject images that are not base64? e.g. i might drag&drop a pdf. Are all files always base64?!
There was a problem hiding this comment.
Only if the file not exists and the file is not a base64, reject.
In additional, I think you should add return before reject, the code should not run through if the file not exists.
There was a problem hiding this comment.
yes, but i mean what happens if you drop a PDF for example. Is that still possible with your changes? Or will it be rejected because it is not bas64?
There was a problem hiding this comment.
This patch does nothing for other file. It will work same as before
There was a problem hiding this comment.
Okay. And why don't you use something like this library instead of implementing it yourself? (tbh: first google result) https://www.npmjs.com/package/fix-orientation ??
There was a problem hiding this comment.
It has many dependencies, maybe we don't need to import so many code.
There was a problem hiding this comment.
I agree with @ehhc. This feature looks good to have. But I think it should not be included in the code base of boostnote. It is almost impossible to maintain...
There was a problem hiding this comment.
Why don't we make a npm module for this? And, I think we need some test for this too.
There was a problem hiding this comment.
Ok, I'll do it some days later.
| @@ -12,6 +12,82 @@ const STORAGE_FOLDER_PLACEHOLDER = ':storage' | |||
| const DESTINATION_FOLDER = 'attachments' | |||
| const PATH_SEPARATORS = escapeStringRegexp(path.posix.sep) + escapeStringRegexp(path.win32.sep) | |||
|
|
|||
| function getImage (file) { | |||
There was a problem hiding this comment.
There is no documentation at all.
Nor are there any tests i see..
There was a problem hiding this comment.
Ok, I will add it later
| }) | ||
| } | ||
|
|
||
| function getOrientation (file) { |
There was a problem hiding this comment.
For me it seems like this method does a lot of black magic.. what are all the constants about? I have no idea how this method works, what it does and what it returns..
There was a problem hiding this comment.
I will add comments later.
| }) | ||
| } | ||
|
|
||
| function fixRotate (file) { |
There was a problem hiding this comment.
How does it "fixes" the "wrong" rotation? I don't understand what might be wrong with it since i cannot reproduce the problem..
There was a problem hiding this comment.
Just like this, https://stackoverflow.com/questions/10600613/ios-image-orientation-has-strange-behavior
This is a demo image.
|
@ehhc comments added. |
|
I cannot replicate the issue. I am on 0.11.9.1 in Ubuntu 16.04 |
|
Try drop this image. It should be landscape. @modmod24 |
|
ok, I see |
Rokt33r
added
next release (v0.11.10)
and removed
awaiting review ❇️
Fix drag-drop image rotate wrong, as Jordan Thornquest mentioned in slack