-
Notifications
You must be signed in to change notification settings - Fork 281
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
Add ShowTypingMiddleware #507
Conversation
Pull Request Test Coverage Report for Build 1761
💛 - Coveralls |
…tor. Update tests
if (context.activity.type === ActivityTypes.Message) { | ||
// Set a property to track whether or not the turn is finished. | ||
// When it flips to true, we won't send anymore typing indicators. | ||
this.finished = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this.finished = false; [](start = 16, length = 22)
What if you have a hundred simultaneous requests? If anything you should store this flag in TurnContext.turnState. You can use a Symbol() for the key to avoid name collisions. Take a look at BotState for an example of that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah whoops!
I've updated this to be a local variable which should resolve the rush condition/state issues.
Any more feedback on this, @Stevenic? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This adds a ShowTypingMiddleware to botbuilder-core, as well as some unit tests.
This is basically a port of the dotnet version, with some slight changes for Javascript.
Should address #470.