Skip to content

Conversation

@alanhoskins
Copy link
Contributor

What kind of change does this PR introduce?

This is part of the requested refactor for hacktoberfest mentioned in #2621.
@Saeris @christianalfoni

What is the current behavior?

Previously it was utilizing inject and hooksObserver from app/componentConnectors.

What is the new behavior?

Uses useOvermind and useEffect to access state and perform checks on isLoggedIn.

What steps did you take to test this? This is required before we can merge.

  1. yarn lint
  2. yarn test
  3. Ran locally, but not sure where this is used.

Checklist

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

I was unable to run yarn contributors:add

@vercel
Copy link

vercel bot commented Oct 14, 2019

This pull request is being automatically deployed with ZEIT Now (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://zeit.co/codesandbox/codesandbox-client/mxv6cwkxw
🌍 Preview: https://codesandbox-cli-git-fork-alanhoskins-refactor-fork-serve-e8524e.codesandbox1.now.sh

@CompuIves
Copy link
Member

CompuIves commented Oct 15, 2019

Heyo! Thanks for the contribution! This can be tested by trying to fork codesandbox.io/s/node (so in localhost) and seeing if the modal pops up and it closes + forks when you sign in.

@christianalfoni
Copy link
Contributor

@alanhoskins Awesome work here! We just found a tiny thing related to importing the component. No wonder, it has not been refactored to Typescript yet :)

Please merge in this PR: https://github.com/alanhoskins/codesandbox-client/pull/1

And we are ready to ship this!

@alanhoskins
Copy link
Contributor Author

@alanhoskins Awesome work here! We just found a tiny thing related to importing the component. No wonder, it has not been refactored to Typescript yet :)

Please merge in this PR: alanhoskins#1

And we are ready to ship this!

All merged. Thanks!

@Saeris Saeris added Hacktoberfest 🔨 Refactor 🧠 Overmind Indicates that this is related to the app's State Management labels Oct 15, 2019
@christianalfoni christianalfoni merged commit 4ecd850 into codesandbox:master Oct 15, 2019
@alanhoskins alanhoskins deleted the refactor-fork-server-modal branch October 15, 2019 12:46
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.

5 participants