I have two buttons
1) Chat button
2) Annotation Button
On tap each view comes from bottom and when tap outside view hidden back (Please refer the screen shot)
On tap On annotation
On tap on Chat
To Handle This I have created following classes
//Annotaion Card
struct CardInfo {
enum CardState {
case expanded
case collpased
}
enum CardType {
case annotaion
case chat
}
var cardType:CardType
var caardHandleHeight:CGFloat
var cardState:CardState = .collpased
var nextState : CardState {
return cardState == .expanded ? .collpased : .expanded
}
mutating func updateToNextState() {
cardState = nextState
}
}
class CardHandler {
var cardInfo:CardInfo
private(set) unowned var viewController : UIViewController
var cardHeight:CGFloat {
switch self.cardInfo.cardType {
case .annotaion:
return UIDevice().userInterfaceIdiom == .pad ? UIScreen.main.bounds.height * 0.4 : 300
case .chat:
return UIDevice().userInterfaceIdiom == .pad ? UIScreen.main.bounds.height * 0.4 : 400
}
}
var isExpanded:Bool {
return cardInfo.cardState == .expanded
}
init (viewController:UIViewController,info:CardInfo) {
self.viewController = viewController
self.cardInfo = info
}
}
In My Main View Controller I have created two properties
lazy var annotationCard = CardHandler(viewController: AnnotaionListViewController.viewController(), info: CardInfo(cardType: .annotaion, caardHandleHeight: 0))
lazy var chatCard = CardHandler(viewController: ChatViewController.viewController(), info: CardInfo(cardType: .chat, caardHandleHeight: 0))
And Add as Child View Controller
func setupCardView() {
visualEffectView = UIVisualEffectView(frame: self.view.bounds)
visualEffectView.effect = UIBlurEffect()
visualEffectView.isUserInteractionEnabled = false
self.view.addSubview(visualEffectView)
let tapGesture = UITapGestureRecognizer(target: self, action: #selector(dismissCardView))
visualEffectView.addGestureRecognizer(tapGesture)
// Add Annotaion View to current class
let annotationVC = self.annotationCard.viewController as! AnnotaionListViewController
annotationVC.closeAction = {
self.createAnimation(card:self.annotationCard, duration: 0.45)
}
annotationVC.annotaionSelected = { annotaion in
self.btnAnnotaion.setImage(annotaion.image, for: .normal)
self.selfSession.selectedAnnotaionType = annotaion
self.createAnimation(card:self.annotationCard, duration: 0.45)
}
self.addChild(annotationVC)
self.view.addSubview(annotationVC.view)
annotationVC.view.frame = CGRect(x:0,y:self.view.frame.height - self.annotationCard.cardInfo.caardHandleHeight,width:self.view.frame.width,height:self.annotationCard.cardHeight)
annotationVC.view.clipsToBounds = true
// Add Chat View to current class
let chatVC = self.chatCard.viewController as! ChatViewController
self.addChild(chatVC)
self.view.addSubview(chatVC.view)
chatVC.view.frame = CGRect(x:0,y:self.view.frame.height - self.chatCard.cardInfo.caardHandleHeight,width:self.view.frame.width,height:self.chatCard.cardHeight)
chatVC.view.clipsToBounds = true
}
// called when tap gesture on Visual effect view
@objc func dismissCardView() {
self.view.endEditing(true)
if self.chatCard.isExpanded {
self.createAnimation(card: self.chatCard, duration: 0.45)
} else if self.annotationCard.isExpanded {
self.createAnimation(card: self.annotationCard, duration: 0.45)
}
}
Now On On tap on button action
@IBAction func btnAnnotationTapped(_ sender: Any) {
self.createAnimation(card:self.annotationCard, duration: 0.45)
}
@IBAction func btnChatTapped(_ sender: Any) {
self.createAnimation(card:self.chatCard, duration: 0.45)
}
Here is method to animate
var animations:[UIViewPropertyAnimator] = []
func createAnimation(card:CardHandler,duration:TimeInterval) {
guard animations.isEmpty else {
print("Animation not empty")
return
}
let viewController = card.viewController
print("array count",self.animations.count)
let cardMoveUpAnimation = UIViewPropertyAnimator.init(duration: duration, dampingRatio: 1.0) { [weak self] in
guard let `self` = self else {return}
switch card.cardInfo.nextState {
case .collpased:
viewController.view.frame.origin.y = self.view.frame.height - card.cardInfo.caardHandleHeight
case .expanded:
viewController.view.frame.origin.y = self.view.frame.height - card.cardHeight
}
}
cardMoveUpAnimation.addCompletion { [weak self] _ in
self?.animations.removeAll()
if card.cardInfo.nextState == .expanded {
self?.view.gestureRecognizers?.map{0ドル.isEnabled = false}
self?.isDrawingEnable = false
self?.visualEffectView.isUserInteractionEnabled = true
} else {
self?.view.gestureRecognizers?.map{0ドル.isEnabled = true}
self?.visualEffectView.isUserInteractionEnabled = false
self?.isDrawingEnable = true
}
card.cardInfo.updateToNextState()
}
cardMoveUpAnimation.startAnimation()
animations.append(cardMoveUpAnimation)
let cornerRadiusAnimation = UIViewPropertyAnimator(duration: duration, curve: .linear) { [weak self] in
switch card.cardInfo.nextState {
case .expanded:
viewController.view.layer.cornerRadius = 12
case .collpased:
viewController.view.layer.cornerRadius = 0
}
}
cornerRadiusAnimation.startAnimation()
animations.append(cornerRadiusAnimation)
let visualEffectAnimation = UIViewPropertyAnimator.init(duration: duration, curve: .linear) { [weak self ] in
switch card.cardInfo.nextState {
case .expanded:
let blurEffect = UIBlurEffect(style: .dark)
self?.visualEffectView.effect = blurEffect
case .collpased:
self?.visualEffectView.effect = nil
}
}
visualEffectAnimation.startAnimation()
animations.append(visualEffectAnimation)
}
I can see there are many issues in code handling & scalability How Can I improve this coding
Please provide suggestions and correction you are seeing in the code
-
\$\begingroup\$ "I can see there are many issues how I have handle this" Can you edit your question please and name these issues specifically. \$\endgroup\$πάντα ῥεῖ– πάντα ῥεῖ2019年11月30日 11:53:57 +00:00Commented Nov 30, 2019 at 11:53
-
\$\begingroup\$ @πάνταῥε What should I name it ? I can see there are many issues how I have handle this ? isn't this too long ? \$\endgroup\$Prashant Tukadiya– Prashant Tukadiya2019年11月30日 11:55:54 +00:00Commented Nov 30, 2019 at 11:55
-
\$\begingroup\$ @πάνταῥεῖ No My code is working fine !! , I want best solution to handle this \$\endgroup\$Prashant Tukadiya– Prashant Tukadiya2019年11月30日 11:57:25 +00:00Commented Nov 30, 2019 at 11:57
2 Answers 2
A few big picture observations:
You really shouldn’t have this much animation code in your view controller.
For example, rather than using view controller containment, I might do a "modal" presentation with a
modalPresentationStyle
of.overCurrentContext
. That gets you out the child view controller containment code. And you can move your animation code into an animator object (see WWDC 2013 video Custom Transitions Using View Controllers). You’ll end up with a radically simplified view controller.When you first do this, it’s going to seem complicated because you’ll be dealing with all sorts of objects with which you are likely unfamiliar, but when you’re done, you’ll end up with an implementation that abstracts the details of how the presentation is performed out of this parent view controller. All of this will be incorporated into specific, animation-related objects and is in keeping with the single responsibility principle.
Anyway, when you’re done, the parent view controller will just be presenting and dismissing the view controllers associated with these two popup views.
The annotation view feels a bit like a keyboard. So it begs the question of whether a proper keyboard might be more appropriate implementation. See https://stackoverflow.com/a/57244936/1271826. Then you get the standard keyboard appearance and disappearance UI for free.
It’s a bit hard to say in this scenario, because we’re not seeing what is shown in the parent view controller and the interaction between these popup views and the main scene, but it’s something to consider.
And now, a bunch of tactical observations:
I’d rename:
CardState.collpased
to be.collapsed
.CardType.annotaion
to.annotation
.annotationVC.annotaionSelected
to.annotationSelected
caardHandleHeight
tocardHandleHeight
selectedAnnotaionType
toselectedAnnotationType
AnnotaionListViewController
toAnnotationListViewController
You have a lot of unnecessary
self
references. I’d personally remove all of them except where they’re absolutely needed (e.g. closures andinit
methods). This eliminates cruft from your code. It also has the virtue of bringing theself
references into stark relief. This prompts us to then more clearly reason about whether you really want strong references orweak
/unowned
references (see next point). But if you have unnecessaryself
references all over the place, these sorts of issues don’t jump out at you like they might otherwise.In your closures, use
weak
orunowned
references (such as inannotationVC.closeAction
orannotationVC.annotationSelected
) because you are likely introducing strong reference cycles. A child object should not have any strong references to the parent.You don’t need the quotes in:
guard let self = self else { return }
Consider:
self?.view.gestureRecognizers?.map { 0ドル.isEnabled = false }
You really should use
forEach
. We usemap
for transforming a sequence of objects into other objects, whereasforEach
is for performing a block of code for each. Clearly we’re not performing any transformation here, soforEach
is appropriate.If this view containment survives your rewrite, make sure to call
didMove(toParent:)
when you’re done adding the subview:addChild(chatVC) view.addSubview(chatVC.view) chatVC.view.frame = ... chatVC.view.clipsToBounds = true chatVC.didMove(toParent: self) // add this line
As the documentation says:
If you are implementing your own container view controller, it must call the
didMove(toParent:)
method of the child view controller after the transition to the new controller is complete ...Make sure you’re in the right coordinate system. When adding a subview, you set its
frame
relative to its superview’sbounds
(not the superview’sframe
). Often you won’t see problems because a view’sframe
andbounds
might be the the same or similar enough in most scenarios, but (a) in certain cases it can cause problems; and (b) it suggests a conceptual misunderstanding of coordinate systems.You have a line that says:
return UIDevice().userInterfaceIdiom == .pad ? UIScreen.main.bounds.height * 0.4 : 300
A couple of problems here:
I think you mean
UIDevice.current
notUIDevice()
.You shouldn’t rely on
UIScreen.main
as you have no assurance that the current view is taking up the whole height of the screen.
Bottom line, you should do you calculations based upon the view height and you should respond to size changes (e.g. in
layoutSubviews
ofUIView
subclass or inviewDidLayoutSubviews
in view controller). Or, better, use constraints instead of manipulating frame settings manually.I personally wouldn’t round the bottom corners of your presented views (with the black background showing through in the lower corners). Sure, on the top it’s fine, but on the bottom it sort of breaks the sliding-from-the-bottom visual metaphor.
Very minor observation, but I’m not sure why
AnnotationListViewController
andChatViewController
have staticviewController
method, rather than just usinginit
(e.g.ChatViewController()
). It’s not a big deal, but seems curious.
-
\$\begingroup\$ For example, rather than using view controller containment, I might do a "modal" presentation with a modalPresentationStyle of .overCurrentContext. --> I am using ARKit (black background in screenshot). So I have paused the session on view will disappear and resume on appears , and annotations are more frequently used so I don't want to use present view controller \$\endgroup\$Prashant Tukadiya– Prashant Tukadiya2019年12月01日 18:13:04 +00:00Commented Dec 1, 2019 at 18:13
-
\$\begingroup\$ When you first do this, it’s going to seem complicated because you’ll be dealing with all sorts of objects with which you are likely unfamiliar... I am sorry I didn't get this point , Could you please explain this ? Do i need to update structure ? \$\endgroup\$Prashant Tukadiya– Prashant Tukadiya2019年12月01日 18:15:03 +00:00Commented Dec 1, 2019 at 18:15
-
\$\begingroup\$ Very minor observation, but I’m not sure why AnnotationListViewController and ChatViewController have static viewController method, rather than just using init (e.g. ChatViewController()). It’s not a big deal, but seems curious --> This will return particular view controller instantiate from stroyboard \$\endgroup\$Prashant Tukadiya– Prashant Tukadiya2019年12月01日 18:18:01 +00:00Commented Dec 1, 2019 at 18:18
-
\$\begingroup\$ "I am using ARKit (black background in screenshot). So I have paused the session on view will disappear and resume on appears , and annotations are more frequently used so I don't want to use present view controller" ... I don’t understand: When you present over the current context and don’t cover the whole context, the effect is identical to what you have here (you still see the view behind where your new view doesn’t cover it). \$\endgroup\$Rob– Rob2019年12月01日 18:21:18 +00:00Commented Dec 1, 2019 at 18:21
-
\$\begingroup\$ "Could you please explain this [custom modal transitions]?" ... See that video that I reference. It walks you through the issues. Or see stackoverflow.com/a/42213998/1271826 or many other S.O. answers about "UIViewController custom transition". \$\endgroup\$Rob– Rob2019年12月01日 18:23:48 +00:00Commented Dec 1, 2019 at 18:23
I would move more logic to an Enum CardInfo
struct CardInfo {
enum CardState {
case expanded
case collapased
var nextState : CardState {
return isExpanded ? .collapased : .expanded
}
var isExpanded: Bool { self == .expanded }
}
enum CardType {
case annotaion
case chat
}
var cardType: CardType
var cardHandleHeight: CGFloat
var cardState: CardState = .collapased
mutating func updateToNextState() {
cardState = cardState.nextState
}
}