Skip to content
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

Allowing storyboard layout & custom main view class for BaseChatViewController #323

Merged
merged 7 commits into from
Jul 26, 2017

Conversation

serge-star
Copy link
Contributor

@serge-star serge-star commented Jun 13, 2017

I've faced a problem trying to subclass an already existing VC from BaseChatViewController.
It had some storyboard layout, and its view was of a custom class with some presentation logic.

However loadView() in BaseChatViewController replaced VC's main view with BaseChatViewControllerView & setupKeyboardTracker() didn't set bmaInputAccessoryView unless it stayed that way.

Now we can work with custom view taken from storyboard:

  1. Change your VC's view class to MyAwesomeCustomViewClass
  2. Make sure MyAwesomeCustomViewClass adapts BaseChatViewControllerViewProtocol
    _
    var bmaInputAccessoryView: UIView?
    override var inputAccessoryView: UIView? {
    return self.bmaInputAccessoryView
    }
    _
  3. In your VC's init() set self.substitutesMainViewAutomatically = false // It has to happen before loadView() is called, init would be a safe place.

@codecov-io
Copy link

codecov-io commented Jun 13, 2017

Codecov Report

❗ No coverage uploaded for pull request base (master@9e8820f). Click here to learn what that means.
The diff coverage is 88.88%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #323   +/-   ##
=========================================
  Coverage          ?   63.46%           
=========================================
  Files             ?       63           
  Lines             ?     3525           
  Branches          ?        0           
=========================================
  Hits              ?     2237           
  Misses            ?     1288           
  Partials          ?        0
Impacted Files Coverage Δ
...ler/Collaborators/BaseChatViewControllerView.swift 0% <ø> (ø)
...Source/ChatController/BaseChatViewController.swift 81.11% <88.88%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9e8820f...9133786. Read the comment docs.

Copy link
Contributor

@AntonPalich AntonPalich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Can you apply requested changes?

@@ -167,7 +176,13 @@ open class BaseChatViewController: UIViewController, UICollectionViewDataSource,
sSelf.isAdjustingInputContainer = false
}
self.keyboardTracker = KeyboardTracker(viewController: self, inputContainer: self.inputContainer, layoutBlock: layoutBlock, notificationCenter: self.notificationCenter)
(self.view as? BaseChatViewControllerView)?.bmaInputAccessoryView = self.keyboardTracker?.trackingView

// Compiler can't understand this cast inline.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is because protocol is not restricted for classes only. Value types can implement it as well. Compiler doesn't allow to modify value type declared with let keyword. Can you fix it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the tip. Fixed.

// Compiler can't understand this cast inline.
// Although we are only setting a property on a constant, it believes that we change reference to it.
// We do not.
if var customView = (self.view as? BaseChatViewControllerViewProtocol) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, remove parentheses

@AntonPalich AntonPalich merged commit fe41493 into badoo:master Jul 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants