2
\$\begingroup\$

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)

enter image description here

On tap On annotation

enter image description here

On tap on Chat

enter image description here

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

asked Nov 30, 2019 at 10:55
\$\endgroup\$
3
  • \$\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\$ Commented 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\$ Commented Nov 30, 2019 at 11:55
  • \$\begingroup\$ @πάνταῥεῖ No My code is working fine !! , I want best solution to handle this \$\endgroup\$ Commented Nov 30, 2019 at 11:57

2 Answers 2

2
\$\begingroup\$

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:

  1. I’d rename:

    • CardState.collpased to be .collapsed.
    • CardType.annotaion to .annotation.
    • annotationVC.annotaionSelected to .annotationSelected
    • caardHandleHeight to cardHandleHeight
    • selectedAnnotaionType to selectedAnnotationType
    • AnnotaionListViewController to AnnotationListViewController
  2. You have a lot of unnecessary self references. I’d personally remove all of them except where they’re absolutely needed (e.g. closures and init methods). This eliminates cruft from your code. It also has the virtue of bringing the self references into stark relief. This prompts us to then more clearly reason about whether you really want strong references or weak/unowned references (see next point). But if you have unnecessary self references all over the place, these sorts of issues don’t jump out at you like they might otherwise.

  3. In your closures, use weak or unowned references (such as in annotationVC.closeAction or annotationVC.annotationSelected) because you are likely introducing strong reference cycles. A child object should not have any strong references to the parent.

  4. You don’t need the quotes in:

    guard let self = self else { return }
    
  5. Consider:

    self?.view.gestureRecognizers?.map { 0ドル.isEnabled = false }
    

    You really should use forEach. We use map for transforming a sequence of objects into other objects, whereas forEach is for performing a block of code for each. Clearly we’re not performing any transformation here, so forEach is appropriate.

  6. 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 ...

  7. Make sure you’re in the right coordinate system. When adding a subview, you set its frame relative to its superview’s bounds (not the superview’s frame). Often you won’t see problems because a view’s frame and bounds 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.

  8. 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 not UIDevice().

    • 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 of UIView subclass or in viewDidLayoutSubviews in view controller). Or, better, use constraints instead of manipulating frame settings manually.

  9. 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.

  10. 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.

answered Dec 1, 2019 at 17:45
\$\endgroup\$
5
  • \$\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\$ Commented 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\$ Commented 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\$ Commented 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\$ Commented 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\$ Commented Dec 1, 2019 at 18:23
1
\$\begingroup\$

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
 }
}
answered Dec 10, 2019 at 14:31
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.