Skip to content

Conversation

@NileshPatel17
Copy link
Contributor

@NileshPatel17 NileshPatel17 commented Oct 4, 2019

What kind of change does this PR introduce?

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

What is the current behavior?

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

What is the new behavior?

Prefers useOvermind over inject

What steps did you take to test this?

  • yarn:lint: There were several linting errors, but none relevant to the scope of this PR
  • yarn:test: Ran successfully

Checklist

  • Documentation
  • Testing
  • Ready to be merged
  • Added myself to contributors table

@vercel vercel bot temporarily deployed to staging October 4, 2019 17:30 Inactive
@vercel
Copy link

vercel bot commented Oct 4, 2019

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

Latest deployment for this branch: https://codesandbox-cl-git-fork-nileshpatel17-refactor-signoutno-a80d33.codesandbox1.now.sh

@vercel vercel bot temporarily deployed to staging October 4, 2019 18:23 Inactive
@NileshPatel17 NileshPatel17 force-pushed the refactor/SignOutNoticeModal branch from 4b4934f to b796fed Compare October 5, 2019 08:36
@vercel vercel bot temporarily deployed to staging October 5, 2019 08:36 Inactive
@Saeris Saeris added Hacktoberfest 🔨 Refactor 🧠 Overmind Indicates that this is related to the app's State Management labels Oct 8, 2019
import { Container, Heading, Explanation } from './elements';

function SignOutNotice({ signals }) {
function SignOutNotice() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
function SignOutNotice() {
export const SignOutNotice = () => {

Let's change this to an arrow function and use a named export here. This will necessitate updating import references across the code base to replace the current default export, but this is our preferred way of handling imports/exports.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed to arrow function. Seems this component has not used yet, hence did not find any references across the code.

}

export default inject('store', 'signals')(observer(SignOutNotice));
export default SignOutNotice;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
export default SignOutNotice;

Don't need this line since we'll be using a named export.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed default import

@CompuIves
Copy link
Member

Thanks for this PR!!

@CompuIves CompuIves merged commit 93b5664 into codesandbox:master Oct 17, 2019
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