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

Moved Typing Indicator to SendBox #2321

Merged
merged 11 commits into from
Aug 19, 2019
Merged

Moved Typing Indicator to SendBox #2321

merged 11 commits into from
Aug 19, 2019

Conversation

tdurnford
Copy link
Contributor

@tdurnford tdurnford commented Aug 15, 2019

Fixes #2214
Fixes #2031

Changelog Entry

Moved the typing indicator to the send box and removed the typing indicator logic from the sagas, by @tdurnford, in PR #2321

Description

Removed typing indicator from the conversation transcript and added it to the SendBox.

Specific Changes

  • Added lastTypingAt reducer that updates on typing activities.
  • Removed TypingActivity component and created TypingIndicator component.
  • Moved typing animation from the activity's assets to the send box's assets.
  • Removed typing indicator logic from sagas
  • Added typingAnimationDuration to the default style options.

....


  • Testing Added

@coveralls
Copy link

coveralls commented Aug 15, 2019

Coverage Status

Coverage increased (+0.3%) to 66.375% when pulling bf5e278 on tdurnford:2214 into 011eae8 on microsoft:master.

@tdurnford tdurnford marked this pull request as ready for review August 16, 2019 16:37
Copy link
Contributor

@corinagum corinagum left a comment

Choose a reason for hiding this comment

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

LGTM. @compulim?

CHANGELOG.md Outdated
@@ -62,6 +62,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
- `component`: Remove [`react`](https://www.npmjs.com/package/react) and [`react-dom`](https://www.npmjs.com/package/react-dom) from `devDependencies`
- `playground`: Remove [`react`](https://www.npmjs.com/package/react) and [`react-dom`](https://www.npmjs.com/package/react-dom) from `dependencies`
- `samples/*`: Move to production version of Web Chat, and bump to [`[email protected]`](https://www.npmjs.com/package/react) and [`[email protected]`](https://www.npmjs.com/package/react-dom)
- Moved the typing indicator to the send box and removed the typing indicator logic from the sagas, by [@tdunrford](https://github.com/tdurnford), in PR [#2321](https://github.com/microsoft/BotFramework-WebChat/pull/2321)
Copy link
Contributor

Choose a reason for hiding this comment

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

spelling OF YOUR OWN USERNAME xP

Copy link
Contributor Author

@tdurnford tdurnford Aug 17, 2019

Choose a reason for hiding this comment

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

Surprised prettier didn't catch that

Copy link
Contributor

@compulim compulim left a comment

Choose a reason for hiding this comment

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

I really love this work.

Simplifying so much code while enabling the user. IMO, this is the most enjoyable part of refactoring/rewriting work.

The direction is great, and I think you will work on the comment. So I am approving this. Feel free to re-request review if big things come up.

__tests__/sendTypingIndicator.js Outdated Show resolved Hide resolved
__tests__/setup/assets/typingIndicator.js Show resolved Hide resolved
__tests__/setup/conditions/receivedTypingActivity.js Outdated Show resolved Hide resolved
__tests__/setup/conditions/receivedTypingActivity.js Outdated Show resolved Hide resolved
__tests__/setup/conditions/receivedTypingActivity.js Outdated Show resolved Hide resolved
packages/core/src/reducers/lastTypingAt.js Outdated Show resolved Hide resolved
packages/core/src/reducers/lastTypingAt.js Outdated Show resolved Hide resolved
packages/core/src/reducers/lastTypingAt.js Outdated Show resolved Hide resolved
packages/core/src/reducers/lastTypingAt.js Outdated Show resolved Hide resolved
packages/core/src/reducers/activities.js Show resolved Hide resolved
@corinagum corinagum merged commit e4ca942 into microsoft:master Aug 19, 2019
@compulim compulim mentioned this pull request Oct 25, 2019
55 tasks
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.

Move typing indicator to send box Need to increase the timeout for TypingAnimation
4 participants