search

chetstone / react-native-palette

A library wrapping the Android Palette class to extract colors from an image.
ISC License
73 stars 8 forks source link

[iOS] support reading Library files instead of assets-library:// #16

Closed cuongchau93 closed 4 years ago

cuongchau93 commented 4 years ago

[refactor] JS part: support dominant color hex to be same key for both ios and android

chetstone commented 4 years ago

Thanks for your contribution! I apologize for not having a Contributing section in my README. I just added one. Would you kindly review my requests there and resubmit the PR? Thanks so much.

chetstone commented 4 years ago

Also one question: does this fix Issue #12 for iOS?

cuongchau93 commented 4 years ago

Thanks for your contribution! I apologize for not having a Contributing section in my README. I just added one. Would you kindly review my requests there and resubmit the PR? Thanks so much.

Sure, will do

Also one question: does this fix Issue #12 for iOS?

No, this does not fix that issue, Currently if we run the example app in ios, and try to load the app local image (the images not in image gallery), the app crashed, The change in this PR is to support that kind of assets

Also in this PR (index.js), I added HEX property in response object for Android, only the first swatches has that HEX property

cuongchau93 commented 4 years ago

the change in this PR breaks the support of library image in ios, because in my case i didnt need it, I will try make it supports both Close the PR for now

cuongchau93 commented 4 years ago

@chetstone before opening another PR, i want to ask if my changes is needed in ios it currently only support asset with path assets-library://asset/asset.JPG?id=B84E8479-475C-4727-A4A4-B77AA9980897&ext=JPG if we supply it with uri file path such as /Users/cuongchau93/Library/Developer/CoreSimulator/Devices/C1105148-CD13-4EB7-98DB-B2A631B5B46E/data/Containers/Data/Application/A298452C-613C-46B0-BC9B-11FDD1475C1B/tmp/DEFA99E7-CA1F-4A77-A16B-4321CDCE3DA8.jpg

it does not work If this change is needed, I will raise another PR

chetstone commented 4 years ago

Hi, thanks very much for responding. Unfortunately I do not have time to look into this this week. I will followup with answers to your questions next week.

Thanks again.

On Sat, May 2, 2020 at 23:04 Cuong Chau notifications@github.com wrote:

@chetstone https://github.com/chetstone before opening another PR, i want to ask if my changes is needed in ios it currently only support asset with path

assets-library://asset/asset.JPG?id=B84E8479-475C-4727-A4A4-B77AA9980897&ext=JPG if we supply it with uri file path such as

/Users/cuongchau93/Library/Developer/CoreSimulator/Devices/C1105148-CD13-4EB7-98DB-B2A631B5B46E/data/Containers/Data/Application/A298452C-613C-46B0-BC9B-11FDD1475C1B/tmp/DEFA99E7-CA1F-4A77-A16B-4321CDCE3DA8.jpg

it does not work If this change is needed, I will raise another PR

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/chetstone/react-native-palette/pull/16#issuecomment-623054639, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAHOLV7SVQEZTEFSWWTZ4YLRPT3NBANCNFSM4MVCPYGQ .