Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Put resources into a bundle #64

Merged
merged 14 commits into from Feb 3, 2015
Merged

Conversation

cabeca
Copy link

@cabeca cabeca commented Nov 25, 2014

Motivation

This MR makes all resources (images and strings) available in a bundle (CTAssetsPickerController.bundle) as opposed to being copied individually into the main bundle. This aims to clean the integration of this pod within the projects by containing all resources inside a bundle instead of scattering them all over the root of the App.

There are three main tasks for achieving this:

  • Create the bundle with the resources inside
  • Make the code load resources from this bundle instead of the main bundle
  • Adapt the demo project to this new reality

Creating the bundle

This was the easy part. Using resource_bundles in pod spec, we can specify which files get inside each bundle. I just created one bundle named CTAssetsPickerController with all the images and localization directories inside. This bundle will be copied to the final product, instead of all the files mentioned in resources

Loading resources from the bundle

To load the resources from this bundle I had to change every resource loading method NSLocalizedStringFromTable() and -[UIImage imageNamed:] to load from the CTAssetsPickerController.bundle instead of the default main bundle. I created two new methods, CTAssetsPickerControllerLocalizedString and - [UIImage ctassetsPickerControllerImageNamed:] that do this. These methods are defined in two categories of UImage and NSBundle.

Adapting the Demo project

The Demo project was changed to use the pod as a library (much like any project using this pod) instead of having the .m files and resources directly in the App target. For that I added a Podfile for the demo project with a reference to the current directory. This will use the podspec in the current directory to build the CTAssetsPickerController.bundle and pod library.
I then removed all CTAssetsPickerController/* files from the demo target because these files will be compiled into the pod library that will be linked to the demo by cocoa pods. Also removed .xcassets and .strings from the demo target because they will be included in the bundle that will be copied to the final product.
I also updated the project settings to the ones recommended by Xcode and added the Pods directory to gitignore.

I know this is a big MR, if you have any doubts please ask me about it.

Thanks!

Miguel Cabeça added 7 commits November 25, 2014 11:15
Added a Podfile for the demo project with a reference to the current
directory. This will use the podspec in the current directory to build
the CTAssetsPickerController bundle.
Removed all CTAssetsPickerController/* files from the demo target.
These files will be compiled into the pod library that will be linked
to the demo by cocoapods.
Updated project settings to the ones recommended by Xcode.
Added the Pods directory to gitignore.
@1and2papa
Copy link
Owner

@cabeca Thanks for your contribution and this comprehensive guide! but I need time to review the changes before merging it.

@1and2papa
Copy link
Owner

@cabeca Hi, I did some test on your code. I agreed that moving all resources into bundle is a right track. But I found that proposed code is mainly targeted for Cocoapods users. It would be broken if developer use the traditional submodules method. Please consider wrapping all picker's resources into a bundle so that it can be work for submodules users.

@cabeca
Copy link
Author

cabeca commented Dec 10, 2014

Hello @chiunam , sorry about not saying something sooner, but I've been somewhat busy lately.
To solve the problem that you mentioned we have two solutions:

  1. Include in the project a shell script to generate the bundle. This shell script would have to be run once by the submodules users. It would mainly create CTAssetsPickerController.bundle directory and copy all the existing *.png and *.lproj into this bundle directory, much like cocoa pods does. This bundle directory would be added to .gitignore. This has the benefit of minimal intrusion and keeping .xcassets directories for easy image management in Xcode.
  2. Create the CTAssetsPickerController.bundle manually and move existing *.png and *.lproj into this bundle directory. The images would have to be moved into this directory and it would not be possible to use .xcassets for image management. Also, *.lproj directories would be "hidden" inside this bundle. The benefit is that no manual script run is needed.

I have a preference ;-) but I would like to know your unbiased opinion on this.

@1and2papa
Copy link
Owner

Hi @cabeca , I've been busy on my own job too. Seems I have to postpone the next major release.

Re the suggested solutions, I didn't realise that it cannot put the .xcassets and *.lproj in the resource bundle. If that's the case, option 1 seems less intrusive to the Xcode project but it still requires developer to pay extra attention on build script... Just wonder if there are any other options?

Cocoapods is good but I still have hesitation to make it as the only option. Though I have no usage statistics of this control, it has already reached 500+ stars on github. Making such huge changes might impact those rely on traditional development methodology. I just want to make it clear on every little thing.

@cabeca
Copy link
Author

cabeca commented Dec 23, 2014

Hello @chiunam

Sorry if I didn't make myself clear, but you can put anything you want inside the resource bundle, it's just a directory with files inside. The problem is that the .xcassets is like a "source file" for images. If you put it inside the resource bundle as is, the imageNamed: methods will not find the relevant images because it is looking at the root of the resource bundle for the images. The *.lproj files would work though.

So, besides options 1. and 2. specified above, I guess there is another option:

3- Include in the project a shell script to generate the bundle. This shell script would have to be run by the authors of this library every time a new resource is added (image or localization). It would mainly create CTAssetsPickerController.bundle directory and copy all the existing _.png and *.lproj into this bundle directory, much like cocoa pods does. This bundle directory would _not* be added to .gitignore and would be the single resource copied to the final product by cocoapods. This has the downside of duplicating resources (both the "source files" and the inside-the-bundle-files) but has the benefit of working for both submodule users and cocoa pods users alike.

I still prefer the option number 1. Cocoapods is all about automation. The submodules approach is all about manual work. I think that submodule users are used to following instructions to use libraries (copy this, configure that, run this script once, etc...). As long as the instructions are clear and in the right "via Git Submodules" section of the README, I don't think that it will be a problem.

But of course that is your call.

@1and2papa
Copy link
Owner

@cabeca Thanks for your explanation. I have a clearer picture now. Please go for option 1 then.

BTW, wish you Merry Christmas and prosperous New Year!

@cabeca
Copy link
Author

cabeca commented Dec 27, 2014

Hello @chiunam

So I went ahead with option 1 and this is the result.

  • There is a new create_bundle script that creates the resource bundle for non cocoapods users. It is a ruby script that reads CTAssetsPickerController.podspec file and, through some ruby magic, extracts the resource_bundles configuration setting and creates the configured bundles, as cocoapods would do itself. If more file types or even more resource bundles are added, this script will continue working correctly. I can explain what the script does in more detail if you want.
  • Updated the README.md with instructions on how to use this script.
  • Cleaned up whitespace in the podspec file
  • Created a new Resources top level directory to contain .xcassets and .lproj resources for the pod. I then moved all .xcassets and .lproj from CTAssetsPickerController to Resources. When a non-cocoapods user runs the create_bundle script, it will create CTAssetsPickerController/CTAssetsPickerController.bundle. In the end this CTAssetsPickerController directory will only contain .m, .h, and CTAssetsPickerController.bundle files that he can drag into his Xcode project.
  • Updated to latest master so it is ready to merge.

@1and2papa
Copy link
Owner

@cabeca Sorry about not responding for a long time. I have been very busy in the past few weeks. I hope this PR can be done in early February. You have my word. :)

@1and2papa 1and2papa merged commit 609eeb5 into 1and2papa:master Feb 3, 2015
1and2papa added a commit that referenced this pull request Feb 3, 2015
#64

Conflicts:
	CTAssetsPickerController.podspec
	CTAssetsPickerDemo.xcodeproj/project.pbxproj
1and2papa added a commit that referenced this pull request Feb 3, 2015
* master:
  #81 Fix: Reload collectionView after calling de/selectAsset methods.
@1and2papa
Copy link
Owner

@cabeca My apologise for leaving it so long. Finally it merges to the master branch and releases as a minor version v2.9.0. I learnt a lot from your contribution. Thank you.

@cabeca
Copy link
Author

cabeca commented Feb 10, 2015

Thank you :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants