-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Happychat: Middleware refactor of HAPPYCHAT_SET_MESSAGE #12565
Happychat: Middleware refactor of HAPPYCHAT_SET_MESSAGE #12565
Conversation
Alphabetize! Alphabetize... again Update middleware helper function to have better name
4a55987
to
2217fe1
Compare
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.
Tested the test plan plus npm run test-client client/state/happychat/test
, and everything worked as expected. Great job!
I've left one comment, but it was soft and shouldn't hold you back 🚢
it( 'should send the connection a typing signal when a message is present', () => { | ||
const action = { type: HAPPYCHAT_SET_MESSAGE, message: 'Hello world' }; | ||
const connection = { typing: spy() }; | ||
middleware( connection )()( spy() )( action ); |
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.
Since we are not really interested in what's happening to the next
function here, could we consider to use noop
from lodash
instead of creating a spy
instance here? It would be a little bit light weighted and the "don't care" intention would be clearer.
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.
Great suggestion 👍
it( 'should send the connection a notTyping signal when the message is blank', () => { | ||
const action = { type: HAPPYCHAT_SET_MESSAGE, message: '' }; | ||
const connection = { notTyping: spy() }; | ||
middleware( connection )()( spy() )( action ); |
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.
The same as the above.
Moves side effects related to
HAPPYCHAT_SET_MESSAGE
actions into the middleware. Part of an ongoing refactor.HAPPYCHAT_SET_MESSAGE
updates the controlled form input state in the Happychat composer when a user types. The side effect is that the remote operator is informed when the user starts and stops typing.To test
happychat
subtree. Themessage
key should initially have a value''
happychat.message
should be syncing in real-time with your changes