Repository navigation
Conversation
|
Can one of the admins verify this patch? |
2 similar comments
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Please sign CLA at http://www.nativescript.org/cla |
|
Updated cla |
|
CLA signature found, happy contributing! |
vakrilov
left a comment
There was a problem hiding this comment.
Can you add a comment in the definitions filethat the value of quality param should be between 1 and 100
| // >> imagesource-to-base-string | ||
| const img = imageSource.fromFile(smallImagePath); | ||
| let base64String = img.toBase64String("png", 80); | ||
| // << imagesource-to-base-string |
| } | ||
|
|
||
| export function testBase64Encode_PNG_WithQuality() { | ||
| // >> imagesource-to-base-string |
| const folder = fs.knownFolders.documents(); | ||
| const path = fs.path.join(folder.path, "test.png"); | ||
| const saved = img.saveToFile(path, "png", 70); | ||
| // << imagesource-save-to |
| } | ||
|
|
||
| export function testSaveToFile_WithQuality() { | ||
| // >> imagesource-save-to |
There was a problem hiding this comment.
Can you remove the // >> ... comments? We add those to extract code snippets that are later used in the docs. They are not related to testing so you don't have to add them.
There was a problem hiding this comment.
Cool, not that makes a lot more sense.
| return false; | ||
| } | ||
|
|
||
| if (quality != null && quality > 1) { |
There was a problem hiding this comment.
Can you change this conditional to just if(quality)?
It's better to always expect the value of quality(if such is provided) to be in the range of 1 to 100.
With this code you will get the same result when passing 0.70 and 70 which can be confusing.
There was a problem hiding this comment.
Yeah I am on it, wasn't sure what to assume so added a logic like this. But I will make the required changes.
| return res; | ||
| } | ||
|
|
||
| if (quality != null && quality > 1) { |
There was a problem hiding this comment.
Can you change this conditional to just if(quality)?
|
Hey @sudhanshu-15, thanks for the awesome PR. |
|
test |
… both platforms Normalize quality in saveToFile and toBase64String to follow 0-100 scale - standardize implementation on both platforms closes NativeScript#5474
…rces update definitions and fix logic of quality in image-sources closes NativeScript#5474
d83d33a to
018ef93
Compare
|
@vakrilov I have updated the changes, please let me know if I have missed something. Thanks a lot for all the help. :) |
|
test |
|
@vakrilov can you tell me where is the build failing? |
|
test |
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Normalize quality in saveToFile and toBase64String to follow 0-100 scale - standardize implementation on both platforms
closes #5474
PR Checklist
What is the current behavior?
quality parameter in image-source follows different scales for both the platforms. On iOS it goes from 0.0-1.0 and on Android it goes from 0-100. Developer needs to manually check for platform when using quality.
What is the new behavior?
iOS implementation of image-source toBase64String and saveToFile now normalizes the quality scale so that both the platforms follow the same scale from 0-100. For iOS the values of 0-100 are converted to a scale of 0.0-1.0.
Fixes/Implements/Closes #[5474].