-
Notifications
You must be signed in to change notification settings - Fork 368
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 - Notification unveil #108
Conversation
Fixes hyperoslo#101 and ensures that 2 line text increases the shouts height
…t fit in the shout notification
@tkohout, thanks for your PR! By analyzing the annotation information on this pull request, we identified @RamonGilabert and @onmyway133 to be potential reviewers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this looks good, just made a few comments about some minor stuff that we probably want to fix up before merging.
Dimensions.height = UIApplication.sharedApplication().statusBarHidden ? 70 : 80 | ||
|
||
Dimensions.height = UIApplication.sharedApplication().statusBarHidden ? 55 : 65 | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you go ahead and remove this whitespace? 😁
@@ -218,15 +220,24 @@ public class ShoutView: UIView { | |||
announcement.action?() | |||
silent() | |||
} | |||
|
|||
|
|||
var subtitleLabelOriginalHeight: CGFloat = 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should probably have this var
at the top of the file instead of in between method declarations.
if panGestureRecognizer.state == .Began { | ||
subtitleLabelOriginalHeight = self.subtitleLabel.bounds.size.height | ||
self.subtitleLabel.numberOfLines = 0 | ||
self.subtitleLabel.sizeToFit() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you need to reference self
here?
@@ -235,14 +246,17 @@ public class ShoutView: UIView { | |||
let height = translation.y < -5 || shouldSilent ? 0 : Dimensions.height | |||
|
|||
duration = 0.2 | |||
|
|||
self.subtitleLabel.numberOfLines = 2 | |||
self.subtitleLabel.sizeToFit() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can probably drop self
here as well.
Thanks for the reply, I will send fixes for this and mentioned minor stuff over the weekend. Cheers |
Done. I added the required changes and fixed the issue with the orientation change. |
@tkohout thanks mate, this is great! |
I have added new feature to unveil rest of the notification if it doesn't fit. Additionally I enlarged the gestureContainer to the whole area of notification. It does work similar as the apple system notifications.
Preview: