cuappdev / tempo

An open-source iOS app for music discovery and sharing with friends.
MIT License
15 stars 1 forks source link

Keivan/music exit #271

Closed ChenJesse closed 7 years ago

ChenJesse commented 7 years ago

Originally this was a simple "disable music on app exit option" task, but we realized that a lot of the front-end architecture was pretty disorganized when we found it very difficult to implement such a simple feature. Therefore, along with that feature is a pretty big refactoring of the entire music-playing flow, as well as animations.

With our addition of the "playerCell" last semester, we can centralize our logic around this new construct. Before, we had a bunch of logic all in a didSet around currentlyPlayingIndex to deal with the pausing/playing of old songs and new, as well as animations. I've been putting more responsibility to playerNav, which is what we call the PlayerNavigationController. playerNav is the primary interface for all the subclasses of PlayerTableViewController to interact with the playerCell and expandedPlayer. It is also the primary reference for stuff like:

The major reason for this organization is the tableViews are not reliable; for example, LikedTableViewController will refresh every time you go back to the view. FeedVC can be manually refreshed. With those 3 pieces of information listed above, we can successfully determine whether a specific cell is the appropriate playing cell after a refresh, and we can then animate as necessary.

Another thing was inheritance; Keivan and I superclassed FeedPostView (originally PostView) and LikedPostView with PostView, which has the methods for updating the animation. We also superclassed FeedTableViewCell and LikedTableViewCell with PostTableViewCell, which has the core characteristic of having a field postView. This just makes the code a lot cleaner.

Finally, the animations were previously very buggy; much of this was because of the reusable cells of tableViews. Now, the logic is simplified:

I think these changes will make adding new features much easier in the future.

I tested a decent amount and everything seems fine to me. I kinda want to merge this in ASAP, since it's a huge PR and it will almost definitely cause everybody merge conflicts. Even if people find new bugs, I would rather merge this in soon and fix them later.