Skip to content

[Added] scrollsToBottomOnKeyboardDidBeginEditing property#217

Merged
SD10 merged 4 commits into
v0.9.0from
enhancement/keyboard-scrolling
Oct 3, 2017
Merged

[Added] scrollsToBottomOnKeyboardDidBeginEditing property#217
SD10 merged 4 commits into
v0.9.0from
enhancement/keyboard-scrolling

Conversation

@SD10

@SD10 SD10 commented Oct 3, 2017

Copy link
Copy Markdown
Member

This closes #203 && #165


fileprivate func addKeyboardObservers() {
NotificationCenter.default.addObserver(self, selector: #selector(handleKeyboardDidChangeState), name: .UIKeyboardWillChangeFrame, object: nil)
NotificationCenter.default.addObserver(self, selector: #selector(handleTextViewDidBeginEditing), name: .UITextViewTextDidBeginEditing, object: messageInputBar.inputTextView)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Out of interest, why are you choosing to add an observer here over assigning the delegate of the textView?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I thought it would be better to not reserve making the delegate our controller

}

fileprivate func removeKeyboardObservers() {
NotificationCenter.default.removeObserver(self, name: .UIKeyboardWillChangeFrame, object: nil)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we need to remove the notification .UITextViewTextDidBeginEditing here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Only for some versions of iOS. Good catch 😉

@SD10 SD10 merged commit 9e3ce4a into v0.9.0 Oct 3, 2017
@SD10 SD10 mentioned this pull request Oct 3, 2017
24 tasks
@SD10 SD10 deleted the enhancement/keyboard-scrolling branch October 3, 2017 22:19
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.

2 participants