Skip to content

Conversation

@nnsnodnb
Copy link
Contributor

@okdtsk
Copy link
Member

okdtsk commented Sep 1, 2017

Thank you for your contribution!

The action field of image carousel template is not list but just json object unlike CarouselTemplate.
ref) https://devdocs.line.me/en/#template-messages

Could you fix your implementation of ImageCarouselColumn and its test?

  • Example json item of columns for image carousel template
{
    "imageUrl": "https://example.com/bot/images/item1.jpg",
    "action": {
    "type": "postback",
    "label": "Buy",
    "data": "action=buy&itemid=111"
  }
}

@nnsnodnb
Copy link
Contributor Author

nnsnodnb commented Sep 2, 2017

Oh, sorry ;(
It is being revised now.

@okdtsk
Copy link
Member

okdtsk commented Sep 4, 2017

Thanks for your updates! :octocat:

I suggest not to separate postback action for standard templates and for image carousel because there is no difference except for the length of label value and our implementation has no validation process to check its length so we can use existing PostbackTemplateAction for image carousel.
Additionally it is better that each action/event has single corresponding class to reduce future maintenance cost of this sdk IMO.

https://devdocs.line.me/en/#template-message

Instead of new implementation of postback action for image carousel, how about modify docstring of existing ***TemplateAction?

@okdtsk okdtsk mentioned this pull request Sep 5, 2017
@bheghenkz
Copy link

bisa tolong buatkan saya bot. anti kicker

@nnsnodnb
Copy link
Contributor Author

nnsnodnb commented Sep 7, 2017

@okdtsk
Sorry...
I fixed it to implementation in PostbackTemplateAction .
I actually implemented it in the application and confirmed the expected behavior.
Remove ImagePostbackTemplateAction .

@okdtsk
Copy link
Member

okdtsk commented Sep 7, 2017

@nnsnodnb
Thank you, LGTM :octocat: I'll merge this pr.

@bheghenkz
If you have a general question of line bot, please ask in https://github.com/line/line-bot-faq. That repository is for general usage of line bot and separate from each sdk repo due to separation of concerns.

@okdtsk okdtsk merged commit ce7d2d2 into line:master Sep 7, 2017
@nnsnodnb nnsnodnb deleted the feature/image_carousel_api branch September 7, 2017 11:45
Renari pushed a commit to Renari/line-bot-sdk-python that referenced this pull request Nov 6, 2017
* Add new api for Image Carousel

* Fix D400 in ImageCarouselColumn

* Remove debug logger

* Fix action array count in ImageCarouselColumn

* Fix ImageCarouselColumn#action to single action

* Fix Image Carousel's test code

* Add ImagePostbackTemplateAction for Image Carousel action

* Fix D204

* Replace ImagePostbackTemplateAction to PostbackTemplateAction & delete former class
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.

3 participants