Skip to content

Conversation

@MihirGH
Copy link
Contributor

@MihirGH MihirGH commented Oct 12, 2019

🔨 Refactored, 🧠 Overmind, Hacktober | Refactor SearchDependenciesModal which now uses useOvermind

What kind of change does this PR introduce?

Refactors code as a part of hacktoberfest mentioned in #2621.
@Saeris @christianalfoni @SaraVieira

What is the current behavior?

app/pages/common/Modals/SearchDependenciesModal/index.js was using inject and hookObserver from app/componentConnectors

What is the new behavior?

Uses useOvermind hook from app/overmind

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

  1. Checked whether the functionality is working or not by opening add dependency modal in sandbox
  2. Ran yarn lint
  3. Ran yarn test

Checklist

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

@vercel
Copy link

vercel bot commented Oct 12, 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/4acf8loz4
🌍 Preview: https://codesandbox-cl-git-fork-mihirgh-refactor-searchdependenc-8739f1.codesandbox1.now.sh

Copy link
Member

@CompuIves CompuIves left a comment

Choose a reason for hiding this comment

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

Nicely done! Thank you!

@CompuIves CompuIves merged commit af5321f into codesandbox:master Oct 12, 2019
@MihirGH MihirGH deleted the refactor/SearchDependenciesModal branch October 12, 2019 16:53
@MihirGH
Copy link
Contributor Author

MihirGH commented Oct 12, 2019

Thank you @CompuIves for the kind words and quick update!
A big thanks to the team for making such a list of components for refactoring and giving opportunity to others for familiarising with the codebase!

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.

3 participants