Skip to content

Conversation

@yevhenorlov
Copy link
Contributor

@yevhenorlov yevhenorlov commented Oct 23, 2019

What kind of change does this PR introduce?

Changing Chat component to use Typescript and Overmind as part of #2621 @Saeris @christianalfoni

What is the current behavior?

Chat component uses inject and observer and does not benefit from types.

What is the new behavior?

Chat component uses useOvermind and Typescript.

What steps did you take to test this? This is required before we can merge, make sure to test the flow you've updated.

  1. yarn lint
  2. yarn test

Wasn't able to open the component locally, so no manual testing unfortunately.

Checklist

  • Documentation N/A
  • Testing
  • Ready to be merged
  • Added myself to contributors table

I ran into some issues with Typescript, also it would be great to have someone take a look at the class-to-functional-component change - in particular, I'm not 100% sure about my ref implementation.

I'll mark the pain points in code additionally.

@lbogdan lbogdan temporarily deployed to pr2910 October 23, 2019 11:33 Inactive
@lbogdan
Copy link
Contributor

lbogdan commented Oct 23, 2019

Build for latest commit d059961 is at https://pr2910.build.csb.dev/s/new.

@lbogdan lbogdan temporarily deployed to pr2910 October 23, 2019 11:56 Inactive
Copy link
Contributor

@christianalfoni christianalfoni left a comment

Choose a reason for hiding this comment

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

Looks great! 👍 Tested:

  • Activate chat
  • Send messages
  • Join new user
  • Send messages from new user
  • Collapse/expand chat
  • Turn off chat

@christianalfoni christianalfoni merged commit 02f678c into codesandbox:master Oct 24, 2019
@yevhenorlov yevhenorlov deleted the pr/chat-hacktoberfest-refactor branch October 24, 2019 10:13
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.

3 participants