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

Feature/add album view #240

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

JohnKuan
Copy link

This pull request is meant to solve https://github.com/ytakzk/Fusuma/issues/29.

There are are substantial changes with regards to how FusumaViewController has to be created.

Firstly, it will require and rely on UINavigationController for the left and right buttons as well as the newly added album selection button. This will no longer require the menuView and the associated buttons with it and hence it was removed in this PR. This will also solve iPhone X layout issue since Apple has made it more applicable to use UINavigationBar, especially to support iPhone X's safe area region.

Next, FusumaViewController has to be wrapped in UINavigationController upon presentViewController method. This is so as to fulfill the requirements in point 1. I have not added a method to create FusumaViewController as a UINavigationController with the current FusumaViewController as rootViewController, since in my use case I can simply call UINavigationController. However, I do understand that it is not wise to give other users the option not to have the UINavigationController in this implementation.

These are the 2 major changes from the existing master branch other than the addition of the album selection table viewcontroller and the respective methods to populate FSAlbumView with the selected PHAssetCollection.

I still believe this is W.I.P and should not be merged at the moment and I would like to hear your feedback with regards to this implementation.

@ytakzk
Copy link
Owner

ytakzk commented Dec 14, 2017

@JohnKuan Resolve conflicts to be merged 🙇

John Kuan added 3 commits December 14, 2017 17:04
…e/add-album-view

* 'master' of https://github.com/ytakzk/Fusuma:
  update swift version
  update podspec
  ytakzk#239 added uitesting
  update -destination
  remove some @objc warnings and prevent crashing in simulator
  update example swift version
  no message
  upgrade xcode image to 9.2
  Swift 4 compatibilty

# Conflicts:
#	Example/FusumaExample.xcodeproj/project.pbxproj
fix errors after merging from master branch
@JohnKuan
Copy link
Author

@ytakzk okay, should be resolved now

@nkezhaya
Copy link
Collaborator

@JohnKuan New maintainer here. Sorry—still just a couple conflicts. I can merge this immediately after they're resolved.

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.

3 participants