Skip to content

Conversation

@yevhenorlov
Copy link
Contributor

What kind of change does this PR introduce?

Changing AddTeamMember component to use Typescript and Overmind, as well as Apollo's new React hook API as part of #2621 @Saeris @christianalfoni

What is the current behavior?

AddTeamMember component uses inject and hooksObserver and does not benefit from types. It also uses legacy render props API for Apollo query.

What is the new behavior?

AddTeamMember component uses useOvermind, Typescript and recommended Apollo hooks API.

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
  3. opened the component at http://localhost:3000/dashboard/teams/:teamId and tested manually.

Checklist

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

@lbogdan
Copy link
Contributor

lbogdan commented Oct 22, 2019

Build for latest commit 73d6585 is at https://pr2906.build.csb.dev/s/new.

@yevhenorlov
Copy link
Contributor Author

not sure why ci/circleci: typecheck fails here - I've reinstalled all the dependencies from scratch, ran yarn lint and yarn test again, everything passes locally 🤔

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:

  • Create team
  • Add team member
  • Accept membership
  • Remove team member

@christianalfoni christianalfoni merged commit 4e483af into codesandbox:master Oct 23, 2019
@yevhenorlov yevhenorlov deleted the pr/add-team-member-hacktoberfest-refactor branch October 23, 2019 11:53
@MichaelDeBoey MichaelDeBoey added 🔨 Refactor 🧠 Overmind Indicates that this is related to the app's State Management labels Feb 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🧠 Overmind Indicates that this is related to the app's State Management 🔨 Refactor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants