Skip to content

Allow MessageLabelDelegate to specify enabled data detectors#197

Closed
hyouuu wants to merge 3 commits into
MessageKit:v0.9.0from
hyouuu:master
Closed

Allow MessageLabelDelegate to specify enabled data detectors#197
hyouuu wants to merge 3 commits into
MessageKit:v0.9.0from
hyouuu:master

Conversation

@hyouuu

@hyouuu hyouuu commented Sep 30, 2017

Copy link
Copy Markdown
Contributor

In my use case, I don't want to show data detectors, and this change allows delegates to list what they want, or simply return nil to keep the default behavior

@SD10

SD10 commented Sep 30, 2017

Copy link
Copy Markdown
Member

@hyouuu I think this is tricky because it's confusing which detectors the cell is going to be using. I do agree that we can improve this. I'm guessing your problem is you want to modify the detectors but it's too difficult for you to subclass?

@hyouuu

hyouuu commented Sep 30, 2017

Copy link
Copy Markdown
Contributor Author

I think most MessageKit users would use a subclass to MessagesViewController e.g.:
class SomeVC: MessagesViewController {}
But MessagesViewController doesn't provide a way to override the data detectors for each message, whereas its MessagesCollectionView var has MessagesLayoutDelegate etc, which I feel is the best place to override this.
Let me know if there is a better way of approaching this or I missed something - thanks!

@SD10

SD10 commented Oct 4, 2017

Copy link
Copy Markdown
Member

@hyouuu I've given this some thought and I don't think it makes sense that this is declared as part of the MessageLabelDelegate. I'm going to close this for now but thank you for making me aware of this limitation. In an upcoming change I will address this and ping you for your input.

@SD10 SD10 closed this Oct 4, 2017
@hyouuu

hyouuu commented Oct 4, 2017

Copy link
Copy Markdown
Contributor Author

Looking forward to it!

@SD10 SD10 mentioned this pull request Oct 4, 2017
1 task
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