make title, message optional. #28
Conversation
|
@dkcas11 please, can you check this PR? |
| @@ -172,7 +171,11 @@ public class CRNotificationView: UIView, CRNotification { | |||
| } | |||
|
|
|||
| @objc internal func dismissNotificationOnTap() { | |||
| delegate?.onNotificationTap(type: type,title: titleLabel.text, message: messageLabel.text) | |||
| guard let onClickDelegate = onClickDelegate else { | |||
| @@ -34,38 +34,42 @@ public class CRNotificationView: UIView, CRNotification { | |||
| label.translatesAutoresizingMaskIntoConstraints = false | |||
| label.font = UIFont.systemFont(ofSize: 13, weight: .semibold) | |||
| label.textColor = .white | |||
| label.numberOfLines = 2 | |||
| label.numberOfLines = 0 | |||
There was a problem hiding this comment.
The purpose of notifications is to inform of lightweight information. If your text cannot fit within 2 lines you are doing something wrong and should use another approach to communicate your information to your user, like an alert.
| @@ -9,7 +9,7 @@ | |||
| import UIKit | |||
|
|
|||
| public protocol CRNotificationDelegate { | |||
| func onNotificationTap(type: CRNotificationType, title: String?, message: String?) | |||
| func onNotificationTap() | |||
There was a problem hiding this comment.
Whats the reason for removing this entirely?
|
|
||
| private var completion: () -> () = {} | ||
| private var type: CRNotificationType | ||
| public var delegate: CRNotificationDelegate? | ||
| public var onClickDelegate: CRNotificationDelegate? |
There was a problem hiding this comment.
Why is the delegate property renamed? There is no point in limiting the delegate property to such a narrow purpose. When you need other delegate methods you will need new specific protocols. It just polutes the codebase.
|
There is simply too big of a change in the API to consider this PR at its current state. Most of the stuff has recently been added by request of other users so this will be a breaking change in many ways. |
add UIStackView that contain titleLabel, messageLabel to handle nil or empty title, message