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

make the parts of state being using more explicit #19968

Merged
merged 1 commit into from Jan 12, 2018

Conversation

Bjvanminnen
Copy link
Contributor

@Bjvanminnen Bjvanminnen commented Jan 12, 2018

This PR is an attempt to make the usage of levelsByLesson a little bit clearer.

Previously, that method was only used by callers that passed in redux state (via. store.progress). Recently, we added a new usage for the progress tab where we pass it an object we've constructed that is a subset of the redux state.

This change makes it a little more explicit what levelsByLesson requires. Rather than just taking a state as input, it takes an object of a specific shape.


I've also been playing around with the idea of starting to use redux for the progress tab, even though we don't necessarily need to. The arguments for doing so would be:
(a) We're already going to have redux on the page (since other tabs need it)
(b) It might be less confusing to have just one way of doing things, rather than sometimes having the data in redux and sometimes having nearly the same data not in redux.

The idea I've been considering, if we were to use redux, would be to extract progress.stages into its own reducer. This would be a really boring reducer, as we initialize it, and then doing nothing else with it. In the case of script overview/puzzle pages, stageReducer would be a child of progressReducer (i.e. we would still have state.progress.stages). In the case of the progress tab, it would be a top level reducer, and we would just have state.stages.

That said, I'm not sure all this really puts things in a more understandable state, so I'm not sure it's worth doing.

@caleybrock
Copy link
Contributor

Code cleanup looks good.
After playing with redux in the manage students tab, I don't think it's a bad idea to set up. In a world with no angular, parts of this page will be in redux anyways, so it might be nice to have all the data there.

@Bjvanminnen Bjvanminnen merged commit 6c2081d into staging Jan 12, 2018
@Bjvanminnen Bjvanminnen deleted the progressTabCleanup branch January 12, 2018 22:19
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.

None yet

2 participants