claudiajs / claudia-bot-builder

Create chat bots for Facebook Messenger, Slack, Amazon Alexa, Skype, Telegram, Viber, Line, GroupMe, Kik and Twilio and deploy to AWS Lambda in minutes
https://claudiajs.com/claudia-bot-builder.html
MIT License
1.84k stars 252 forks source link

Skype format message #58

Closed IvanJov closed 7 years ago

IvanJov commented 7 years ago

Please review the code!

I would like to finish remaining tasks bellow before merging.

stojanovic commented 7 years ago

Seem good so far, just a question, is any info required for Hero, Thumbnail and Receipt? Not sure I like this structure:

.addHero(title, subtitle, text, images)

if nothing is required, because this doesn't look that nice:

.addHero(null, null, null, [imageUrl])

if only images are required I would prefer:

.addHero(images, title, subtitle, text)

or even:

.addHero(images, text)
  .addTitle(title)
  .addSubtitle(subtitle)
IvanJov commented 7 years ago

@stojanovic Nothing is required for Hero, Thumbnail and Receipt. I like the idea for addTitle and addSubtitle functions.

What do you think about passing and object with title, subtitle, text and image to addHero (or other) function?

stojanovic commented 7 years ago

We already have something like .addTitle in FB templates

Object works too

IvanJov commented 7 years ago

@stojanovic Ok, I am going with functions!

IvanJov commented 7 years ago

This is fixed. I will write documentation and this can probably be merged. I will work later on the remaining two points I posted above.

IvanJov commented 7 years ago

@stojanovic Just wrote documentation. Please let me know if you think that something is missing. I will keep updating working on the Skype format messages, need to check two remaining stuffs I wrote above, but for now I think that this is ready for merge.

I've tested all methods in Skype bot.

stojanovic commented 7 years ago

Cool, thanks!