Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bind action creators to dispatch for page level components and remove appState #1203

Merged
merged 2 commits into from
Aug 15, 2017

Conversation

jeremiak
Copy link
Contributor

Use the pattern from <App /> to bind action creators to dispatch (so that we don't need to pass dispatch into the props of a component) in the page level components (/src/pages) for consistency.

Also remove appState so that the page level components can use connect() to pull the relevant parts of the application state for themselves.

Copy link
Contributor

@jpwentz jpwentz left a comment

Choose a reason for hiding this comment

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

Looks good.

Jeremia Kimelman added 2 commits August 15, 2017 00:06
Removing the dispatch prop in favor of an object called actions that is
the necessary action creators bound with the dispatch function by the
time they are passed into the component.

Also reduce the appState prop that is passed in, particularly to
<Explorer /> so that page level components can use connect() to get the
parts of app state that they care about and it isn't passed in from
higher props.
Passing around appState is sort of annoying because it isn't that
descriptive and it really only came from <App />. Now that we have a
separate page level component directory, most of them are connected to
the store and therefore can pull the relevant parts from appState for
themselves.
@jeremiak jeremiak force-pushed the jk-bind-action-creators branch from c27ac84 to 319f400 Compare August 15, 2017 04:06
@jeremiak jeremiak merged commit 7d82da1 into master Aug 15, 2017
@jeremiak jeremiak deleted the jk-bind-action-creators branch August 15, 2017 04: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.

2 participants