Skip to content
This repository has been archived by the owner on Jul 9, 2022. It is now read-only.

Added Mentions #460

Merged
merged 3 commits into from
Apr 12, 2017
Merged

Added Mentions #460

merged 3 commits into from
Apr 12, 2017

Conversation

MikeShi42
Copy link
Contributor

Added the ability to use mentions in the sendMessage object so that a bot can ping specific people.

Main issue is that tag location is determined with indexOf which is a bit scrappy. It provides a good balance of ease of use as the tag text can just be passed in, instead of the user having to figure out where the tag is in the string themselves. Can be changed to also explicitly take in beginning + ending index of the string as well.

@Schmavery
Copy link
Owner

Schmavery commented Apr 10, 2017

Awesome work @MikeShi42 !
What do people think about an optional fromIndex like is offered by javascript's indexOf operator? https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/indexOf

It could default to 0 and that way people still have all the power they need just in case.

@Schmavery
Copy link
Owner

Schmavery commented Apr 10, 2017

Also: Is there any verification of input we can do? i.e. checking if msg.body.indexOf(tag) is -1 and if so calling the callback with an error? I'd probably prefer this to returning some generic facebook error.

@MikeShi42
Copy link
Contributor Author

@Schmavery we can def do some client side checking as well (sorry the code is hackish). It seems like FB will happily take the invalid input and just toss it out when rendering in terms of errors, it would be nice for the library to tell the user that their mention is 'invalid'. As for the fromIndex, are you thinking of it being an optional property in the passed in object? It seems like a good idea, and along that lines we can also include a toIndex for more flexibility.

Additionally, how do you feel about the documentation? I know the API I've created is a bit bulkier than other fields so I had some trouble trying to explain it well and just attached an example which hopefully would clear things up.

@Schmavery
Copy link
Owner

I think the example works well for docs. You can make a note in the description of the field to see below for an example (bonus points for making it link).

I think it makes sense to just add the fromIndex (toIndex is not really necessary and doesn't match the signature of indexOf). Adding it as an optional field in the passed in objects (default = 0) would be great.

Yeah basically some error or warning when msg.body.indexOf(tag, fromIndex) < 0 or something would be great.

const mention = msg.mentions[i];

const tag = mention.tag;
if (typeof tag !== "string")
Copy link
Owner

Choose a reason for hiding this comment

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

can you add curly braces :D

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yup c:

@Schmavery
Copy link
Owner

Thanks @MikeShi42 !

@Schmavery Schmavery merged commit 413a517 into Schmavery:master Apr 12, 2017
how2945ard pushed a commit to how2945ard/facebook-chat-api that referenced this pull request May 30, 2017
* Added mentions sendMessage handler + docs.
* Mentions: Added fromIndex, checks and warnings
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants