Finished the full view documentation #55
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Started adding in the documentation for the sort functions and finished the documentation for all of the full view animation extensions.
Found a small memory leak in the library that needed to be fixed. Turned out that Spruce was holding onto the objects so we needed to make sure that it was a weak reference. If that goes away then we will simply skip it.
Currently we don't have time to allow the example app to support landscape orientation. To fix this, we will just lock it to portrait. Down the road this is something we plan to change.
On smaller devices the start selector was not being shown fully. Added this selector so that it does onto the next line rather than cutting off the title.
mattyohe
reviewed
Mar 7, 2017
| @@ -93,6 +100,9 @@ public extension UIView { | |||
| } | |||
| } | |||
|
|
|||
| /* | |||
| As a nicety, if the view is fading in then we know it does not need to be visible. So let's hide the view so that it does not intercept any touch events while it is transparent. | |||
There was a problem hiding this comment.
This comment seems to state the opposite of what happens later?
if the view is fading in then we know it does not need to be visible.
So you setisFadingto true.
let's hide the view
It appears on line 119 you check to see ifisFadingis true, and then if it is, I expect you to hide the view. You however setisHiddento false?
| @@ -93,6 +100,9 @@ public extension UIView { | |||
| } | |||
| } | |||
|
|
|||
| /* | |||
| As a nicety, if the view is fading in then we know it does not need to be visible. So let's hide the view so that it does not intercept any touch events while it is transparent. | |||
There was a problem hiding this comment.
intercept any touch events while it is transparent.
I don't believe UIKit allows a transparent view to receive touches.
Rather than using a label inside a scroll view to try and remove the wrapping, placed the code inside a unscrollable . This allows the text to be wrapped and also copied.
Since there were quite a few comments, it made it easier to put them all in one pull request. This way we could both avoid merge conflicts and try to get everything so that nothing is left out. In this PR, I renamed all of the `Spruce` methods so that they don't include the prefix.
Since `Swift` extensions are essentially categories in objective-c, we need to make sure that we prefix each of the methods with an identifying symbol. To do so, we changed methods such as `spruceUp` to `spruce_up` so that the prefix is `spruce_`. Anywhere we have a extended method this prefix is attached. The question is whether or not `spruce_` is a good prefix. Whether we should keep it or change it? And with this new prefix should we change any of the method names so that they make more sense. We were playing around with the idea of `spruce_animate` instead of `spruce_up`. Let us know your thoughts :)
This PR will rename the methods back to their initial state. Also changes around some enums so that their values make more sense lyrically.
In order to avoid conflicts with namespaces, we took the computed variable approach. This would entail creating a `Spruce` struct that would encapsulate a `UIView`. This way you would simply call `view.spruce.animate` rather than having to use `view.spruceAnimate`. This looks a little bit cleaner and will show all methods under that one variable.
jacksontaylor13
changed the base branch from
documentation/sort-functions-documentation-2
to
develop
…pproach Add computed variable approach to UIView
…-prefix Update naming scheme to prefix extension methods
Respond to PR Comments
Implement wrapping for the code preview
Update view layout for start selector
Fix issues with Memory Leak
…ns-documentation-4 Complete Documentation
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Started adding in the documentation for the sort functions and finished the documentation for all of the full view animation extensions.