Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue #359 .reprompt() not concatenating #364

Merged
merged 5 commits into from
Sep 10, 2018
Merged

Conversation

andrewjhunt
Copy link

@andrewjhunt andrewjhunt commented Aug 17, 2018

Fixes #359
https://github.com/alexa-js/alexa-app/blob/master/index.js#L51

Example: the following will speak "1 2" as output.
response.reprompt("1").reprompt("2")

Existing unit tests are updated and pass.

NOTE: the doc is not explicit on the expected behavior of multiple reprompts for the same response. I expected that multiple reprompt() would also concatenate as say(). The existing unit tests didn't reflect this but it's not clear that was intentional. Plus the code appeared to reference a non-defined variable.

https://github.com/alexa-js/alexa-app/blob/master/index.js#L51

Example: the following will speak "1 2" as output.

response.reprompt("1").reprompt("2")
@kobim
Copy link
Contributor

kobim commented Aug 19, 2018

Hi @andrewjhunt, can you please update the CHANGELOG and also add a note about the concatenation of reprompt()?
Other than that, it looks good to me.

@kobim kobim self-requested a review August 19, 2018 09:31
Copy link
Contributor

@kobim kobim left a comment

Choose a reason for hiding this comment

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

Can you please also add hints for this concatenation option in both README.md and index.d.ts?
You can see how say() hints about it.

CHANGELOG.md Outdated
@@ -5,7 +5,7 @@
* [#344](https://github.com/alexa-js/alexa-app/pull/344): Add coveralls as a check for pull requests - [@kobim](https://github.com/kobim).
* [#352](https://github.com/alexa-js/alexa-app/pull/352): Allow utterance expansion in custom slot type synonyms - [@daanzu](https://github.com/daanzu).
* [#361](https://github.com/alexa-js/alexa-app/pull/361): Fix object reference for dialogState in in doc - [@Sephtenen](https://github.com/Sephtenen).
* Your contribution here.
* [#364](https://github.com/alexa-js/alexa-app/pull/364): Fix reprompt() to concatenate multiple SSML prompts - [@andrewjhunt](https://github.com/andrewjhunt).
Copy link
Contributor

Choose a reason for hiding this comment

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

Please re-add the deleted "* Your contribution here." beneath.

@andrewjhunt
Copy link
Author

added reprompt() multiple use doc to README.md and index.d.ts

types/index.d.ts Outdated Show resolved Hide resolved
As requested by kobim
@kobim kobim merged commit 4e67d3d into alexa-js:master Sep 10, 2018
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