Projects gallery: reset gallery state #15621
Conversation
| import {registerReducers} from '@cdo/apps/redux'; | ||
|
|
||
| // Actions | ||
| const TOGGLE_GALLERY = 'Projects/TOGGLE_GALLERY'; |
There was a problem hiding this comment.
another minor style difference: that gamelab is using camel case like Projects whereas our other code uses lowercase like projects. Again this is fine to ignore since it'd be easy to change later if we decide to.
| @@ -0,0 +1,30 @@ | |||
| /** @file Redux actions and reducer for the Projects Gallery */ | |||
There was a problem hiding this comment.
just noting that the filename here follows the style used in gamelab, whereas we use redux.js in most other parts in apps. I think this is fine and should be easier to switch later if we decide to standardize.
| @@ -50,31 +48,29 @@ const styles = { | |||
|
|
|||
| class GallerySwitcher extends Component { | |||
| static propTypes = { | |||
| initialGallery: PropTypes.oneOf(Object.keys(Galleries)).isRequired, | |||
| pageLocation: PropTypes.string.isRequired, | |||
There was a problem hiding this comment.
the name pageLocation seems misleading since it suggests a URL path like /projects or /projects/public. What do you think about calling this selectedGallery, or anything else without "location" in it? If you do change it, the string should probably be changed everywhere it appears in this PR.
There was a problem hiding this comment.
I started building on top of this PR and now i have a few more requests :) I couldn't figure out what kind of thing selectGallery was from reading our code, so I went to look it up in the redux docs and now there are a couple of places I think it would be helpful to add or change code comments in projectsModule.js.
| /** @file Redux actions and reducer for the Projects Gallery */ | ||
| import {registerReducers} from '@cdo/apps/redux'; | ||
|
|
||
| // Actions |
There was a problem hiding this comment.
According to http://redux.js.org/docs/basics/Actions.html these are "Action types".
| */ | ||
| export function selectGallery(projectType) { | ||
| return { type: TOGGLE_GALLERY, projectType: projectType }; | ||
| } |
There was a problem hiding this comment.
According to http://redux.js.org/docs/basics/Actions.html these are "Action creators".
| selectGallery(gallery){ | ||
| dispatch(selectGallery(gallery)); | ||
| } | ||
| }))(Radium(GallerySwitcher)); |
There was a problem hiding this comment.
connect actually has a short-form, where if you're just dispatch actions with the same props, I believe you can do this:
connect({state => ({
pageLocation: state.pageLocation
}), { selectGallery });
|
|
||
| // Reducer | ||
| function pageLocation(state, action) { | ||
| const defaultState = isPublic ? 'PUBLIC' : 'PRIVATE'; |
There was a problem hiding this comment.
I would rename defaultState to initialState and move it outside of the reducer. Then you can do
const initialState = isPublic ? 'PUBLIC' : 'PRIVATE';
function pageLocation(state=initialState, action) {
// action handling here
}There was a problem hiding this comment.
Thinking about this some more, I think I would move the isPublic check out of this module and have a caller set the state based on window.location. This module itself should not depend on window.location.
caleybrock
force-pushed
the
reset-gallery-state
branch
from
9388515
to
b5b56bf
Compare
UX fix: If I click on “View more ___ Projects,” go back to My projects list, and then go back to the Public Gallery, I should be taken back to the main gallery page (not stay in the specific projects page)
This PR sets up redux in the project gallery and uses it to store the page location.