Skip to content

Fix send typing#1541

Merged
compulim merged 12 commits intomicrosoft:masterfrom
compulim:fix-send-typing
Jan 16, 2019
Merged

Fix send typing#1541
compulim merged 12 commits intomicrosoft:masterfrom
compulim:fix-send-typing

Conversation

@compulim
Copy link
Copy Markdown
Contributor

@compulim compulim commented Dec 24, 2018

Fix #1539

Background

Send typing was not working because:

  • We didn't read initial sendTyping props correctly
    • Missing sendTyping reducer
  • We didn't clear out typing activity correctly
    • Should not add self typing activity
    • Should use takeEvery instead of takeLatest

Also added a sample to show the send typing behavior.

@compulim compulim added bug Indicates an unexpected problem or an unintended behavior. 4.3 labels Dec 24, 2018
@compulim compulim added the p1 Painful if we don't fix, won't block releasing label Dec 24, 2018
@coveralls
Copy link
Copy Markdown

coveralls commented Dec 24, 2018 •
edited
Loading

Pull Request Test Coverage Report for Build 764

  • 8 of 16 (50.0%) changed or added relevant lines in 4 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.06%) to 47.949%
Changes Missing Coverage Covered Lines Changed/Added Lines %
packages/core/src/reducers/activities.js 1 2 50.0%
packages/core/src/sagas/sendTypingOnSetSendBoxSaga.js 0 7 0.0%
Totals Coverage Status
Change from base Build 763: 0.06%
Covered Lines: 784
Relevant Lines: 1475
💛 - Coveralls

cwhitten
cwhitten reviewed Dec 26, 2018
View reviewed changes
Comment thread CHANGELOG.md Outdated
corinagum
corinagum suggested changes Dec 31, 2018
View reviewed changes
Copy link
Copy Markdown
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.

Pending naming discussion

Comment thread README.md Outdated
Comment thread samples/07.b.customization-send-typing/index.html Outdated
@compulim
Copy link
Copy Markdown
Contributor Author

compulim commented Jan 13, 2019

@cwhitten @corinagum I have updated from "send typing" to "send typing indicator".

I only renamed the sample for now. Will submit another PR for renaming actions, reducers, sagas, selectors, props, and add deprecation notes.

@compulim compulim mentioned this pull request Jan 13, 2019
Merged
@corinagum
Copy link
Copy Markdown
Contributor

corinagum commented Jan 14, 2019

I approved, but just in case wait for confirmation from @cwhitten :)

@compulim compulim merged commit b341439 into microsoft:master Jan 16, 2019
@compulim compulim deleted the fix-send-typing branch January 16, 2019 03:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cwhitten cwhitten cwhitten approved these changes

@justinwilaby justinwilaby Awaiting requested review from justinwilaby

@tonyanziano tonyanziano Awaiting requested review from tonyanziano

@a-b-r-o-w-n a-b-r-o-w-n Awaiting requested review from a-b-r-o-w-n a-b-r-o-w-n is a code owner

+1 more reviewer

@corinagum corinagum corinagum approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

bug Indicates an unexpected problem or an unintended behavior. p1 Painful if we don't fix, won't block releasing

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@compulim @coveralls @corinagum @cwhitten