exyte / fan-menu

Menu with a circular layout based on Macaw
MIT License
728 stars 58 forks source link

Fix Carthage dependencies and images in other bundles #27

Closed zocario closed 5 years ago

zocario commented 5 years ago

Add explicit dependencies for carthage through Cartfile

This fix issue when adding fan-menu to a project managing dependencies with carthage and not using Macaw yet. It allows carthage to detect Macaw as needed dependency and fixes build error when installing fan-menu.

Change FanMenuButton’s image string to UIImage

This allows to pass image from different bundle than the main bundle of the App.

amarunko commented 5 years ago

Hi, @zocario, thanks for your help! Could you please divide features on different pull requests in the future. Also, why did you add cartfile?

zocario commented 5 years ago

Hi @amarunko yes sorry I did it one time as I'm using this branch for a project.. I added Cartfile because if you have a project managing dependencies with Carthage and not using Macaw you'll not be able to compile fan-menu. Adding this file allows Carthage to understand we need to add Macaw as a dependency and compile it before compiling fan-menu.

zocario commented 5 years ago

As an example you can see with ReactiveCocoa: https://github.com/ReactiveCocoa/ReactiveCocoa It has a cartfile indicating it uses reactive swift so when we add it to our cartfile in final App we don't need to manually add it's internal dependencies in our cartfile, but it'll be deduced by Carthage and added in cartfile.resolved.

amarunko commented 5 years ago

@zocario, understood, but it broke functional when you need both - Macaw and Fan-Menu, please send us pull request with image changes individually - it will be merged asap.

zocario commented 5 years ago

@amarunko Just created a new one only for images https://github.com/exyte/fan-menu/pull/28. But to be honest I'm not sure to understand what you mean by it broke functional when you need both - Macaw and Fan-Menu?

amarunko commented 5 years ago

So, some time ago we did have one problem. If we have a project that has two dependencies - Macaw and FanMenu in one time, if FanMenu has cartfile with Macaw dependency - Carthage not work properly. That's why we remove cartfile some time ago. You can find discussion here.

zocario commented 5 years ago

I close this PR as image fix was merged in #28 and a new suggestion to fix Carthage configuration is proposed in #29