Plugin Support for MessageInputBar#2
Conversation
Refinements and Consistency Changes
|
Its a lot of lines but mostly documentation and views. Core logic in AttachmentManager and AutocompleteManager. AutocompleteManager was vetted by MessageViewController (Githawk) |
|
@nathantannar4 This looks good to me, a few questions though:
|
|
We should also add to the README that this project is not being used in MessageKit yet, it would be great if we could replace it in the next ~1.5 months though. This Summer for sure. Unit tests would give me more confidence of course. This is a 2.0 feature |
|
This is pretty much the last of the changes. The merged PRs in your referenced issues are in MessageInputBar. Sent with GitHawk |
|
We should definitely have MessageKit using this repo for the v1.0 otherwise there could be name conflicts for people trying to use both Sent with GitHawk |
|
@nathantannar4 Good point on the naming conflicts 🤔 This is something I’ll think about this week Sent with GitHawk |
|
@nathantannar4 So this looks pretty safe to support |
|
Any API breaking changes I can revert Sent with GitHawk |
| import UIKit | ||
|
|
||
| /// `InputPlugin` is a protocol that makes integrating plugins to the `MessageInputBar` easy. | ||
| public protocol InputPlugin: class { |
|
|
||
| } | ||
|
|
||
| extension NSAttributedString { |
There was a problem hiding this comment.
Can we explicitly mark all helper extensions in the library as internal?
| import UIKit | ||
|
|
||
| /// AutocompleteManagerDelegate is a protocol that more precisely define AutocompleteManager logic | ||
| public protocol AutocompleteManagerDelegate: class { |
| import UIKit | ||
|
|
||
| /// AutocompleteManagerDataSource is a protocol that passes data to the AutocompleteManager | ||
| public protocol AutocompleteManagerDataSource: class { |
| } | ||
|
|
||
| extension AutocompleteManager: UITableViewDelegate, UITableViewDataSource { | ||
|
|
There was a problem hiding this comment.
It may be a better approach to stop using this pattern of confirming to a protocol via extension. This doesn't allow users to override any of the methods declared here
| // range.length > 0: Backspace/removing text | ||
| // range.lowerBound < textView.selectedRange.lowerBound: Ignore trying to delete | ||
| // the substring if the user is already doing so | ||
| if range.length > 0, range.lowerBound < textView.selectedRange.lowerBound { |
There was a problem hiding this comment.
I think I fixed some bugs in GitHawk related to this logic 🤔 but I'm fine with leaving it for now. we will need unit tests one day
There was a problem hiding this comment.
This is copied from the githawk fixes
| import Foundation | ||
|
|
||
| /// AttachmentManagerDataSource is a protocol to passes data to the AttachmentManager | ||
| public protocol AttachmentManagerDataSource: class { |
There was a problem hiding this comment.
AnyObject, anywhere to constrain a protocol really
There was a problem hiding this comment.
What's the benefit just curious?
There was a problem hiding this comment.
@nathantannar4 no clue, it's what the Swift Programming Language Guide book shows when constraining a protocol to be class only, so this is what I've started using
|
@nathantannar4 Please make these changes when you can so we can do |
Forked the plugins I made in InputBarAccessoryView and make them subspecs for MessageInputBar