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

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

| improve this answer | |
\$\endgroup\$
  • \$\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 Dec 1 '19 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 Dec 1 '19 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 Dec 1 '19 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 Dec 1 '19 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 Dec 1 '19 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
    }
}
| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.