From 7d84783b40b79de4e777fab7df9cc5b4c6f6dcb0 Mon Sep 17 00:00:00 2001 From: Oliver Eyton-Williams Date: Fri, 30 Aug 2019 19:15:26 +0200 Subject: [PATCH] feat: show last viewed challenge in curriculum map on reload (#35421) * feat: Curriculum page loads with current challenge block open * feat: Curriculum page expansion changes from review * feat: Curriculum page expansion changes from second review * fix: use a cookie to track current challenge * fix: remove redundant server changes * fix: add PropTypes to fix lint error * fix: change cookies to store and add tests * fix: use currentChallengeId * fix: update tests I couldn't figure out how to test the challenge saga, so that's gone for now. The Map tests have been updated to use currentChallengeId * fix: remove unused PropTypes * fix: separate currentChallengeId from user * fix: update currentChallengeId directly on mount * feat: reset map on every visit --- client/src/components/Map/Map.js | 55 ++++++++++- client/src/components/Map/Map.test.js | 95 ++++++++++++++++++- client/src/components/Map/components/Block.js | 7 +- client/src/components/Map/redux/index.js | 21 ++-- client/src/redux/index.js | 18 +++- .../redux/current-challenge-saga.js | 12 ++- 6 files changed, 181 insertions(+), 27 deletions(-) diff --git a/client/src/components/Map/Map.js b/client/src/components/Map/Map.js index e67608db5d..fda9140dd6 100644 --- a/client/src/components/Map/Map.js +++ b/client/src/components/Map/Map.js @@ -1,14 +1,20 @@ import React, { Component } from 'react'; +import { connect } from 'react-redux'; +import { bindActionCreators } from 'redux'; import PropTypes from 'prop-types'; import uniq from 'lodash/uniq'; +import { createSelector } from 'reselect'; import SuperBlock from './components/SuperBlock'; import Spacer from '../helpers/Spacer'; import './map.css'; import { ChallengeNode } from '../../redux/propTypes'; +import { toggleSuperBlock, toggleBlock, resetExpansion } from './redux'; +import { currentChallengeIdSelector } from '../../redux'; const propTypes = { + currentChallengeId: PropTypes.string, introNodes: PropTypes.arrayOf( PropTypes.shape({ fields: PropTypes.shape({ slug: PropTypes.string.isRequired }), @@ -18,10 +24,46 @@ const propTypes = { }) }) ), - nodes: PropTypes.arrayOf(ChallengeNode) + nodes: PropTypes.arrayOf(ChallengeNode), + resetExpansion: PropTypes.func, + toggleBlock: PropTypes.func.isRequired, + toggleSuperBlock: PropTypes.func.isRequired }; -class ShowMap extends Component { +const mapStateToProps = state => { + return createSelector( + currentChallengeIdSelector, + currentChallengeId => ({ + currentChallengeId + }) + )(state); +}; + +function mapDispatchToProps(dispatch) { + return bindActionCreators( + { + resetExpansion, + toggleSuperBlock, + toggleBlock + }, + dispatch + ); +} + +export class Map extends Component { + componentDidMount() { + this.initializeExpandedState(this.props.currentChallengeId); + } + + initializeExpandedState(currentChallengeId) { + this.props.resetExpansion(); + const { superBlock, block } = currentChallengeId + ? this.props.nodes.find(node => node.id === currentChallengeId) + : this.props.nodes[0]; + this.props.toggleBlock(block); + this.props.toggleSuperBlock(superBlock); + } + renderSuperBlocks(superBlocks) { const { nodes, introNodes } = this.props; return superBlocks.map(superBlock => ( @@ -48,7 +90,10 @@ class ShowMap extends Component { } } -ShowMap.displayName = 'Map'; -ShowMap.propTypes = propTypes; +Map.displayName = 'Map'; +Map.propTypes = propTypes; -export default ShowMap; +export default connect( + mapStateToProps, + mapDispatchToProps +)(Map); diff --git a/client/src/components/Map/Map.test.js b/client/src/components/Map/Map.test.js index 69714a9c80..3b3479f8be 100644 --- a/client/src/components/Map/Map.test.js +++ b/client/src/components/Map/Map.test.js @@ -1,20 +1,105 @@ -/* global expect */ +/* global expect jest */ import React from 'react'; import ShallowRenderer from 'react-test-renderer/shallow'; -import Enzyme from 'enzyme'; +import Enzyme, { shallow } from 'enzyme'; import Adapter from 'enzyme-adapter-react-16'; +import store from 'store'; -import Map from './Map'; +import { Map } from './Map'; import mockNodes from '../../__mocks__/map-nodes'; import mockIntroNodes from '../../__mocks__/intro-nodes'; Enzyme.configure({ adapter: new Adapter() }); const renderer = new ShallowRenderer(); +const baseProps = { + introNodes: mockIntroNodes, + nodes: mockNodes, + toggleBlock: () => {}, + toggleSuperBlock: () => {}, + resetExpansion: () => {} +}; + test(' snapshot', () => { - const component = renderer.render( - + const componentToRender = ( + {}} + toggleSuperBlock={() => {}} + /> ); + const component = renderer.render(componentToRender); expect(component).toMatchSnapshot('Map'); }); + +describe('', () => { + describe('after reload', () => { + let initializeSpy = null; + beforeEach(() => { + initializeSpy = jest.spyOn(Map.prototype, 'initializeExpandedState'); + }); + afterEach(() => { + initializeSpy.mockRestore(); + store.clearAll(); + }); + // 7 was chosen because it has a different superblock from the first node. + const currentChallengeId = mockNodes[7].id; + + it('should expand the block with the most recent challenge', () => { + const blockSpy = jest.fn(); + const superSpy = jest.fn(); + const props = { + ...baseProps, + toggleBlock: blockSpy, + toggleSuperBlock: superSpy, + currentChallengeId: currentChallengeId + }; + const mapToRender = ; + shallow(mapToRender); + expect(blockSpy).toHaveBeenCalledTimes(1); + expect(blockSpy).toHaveBeenCalledWith(mockNodes[7].block); + + expect(superSpy).toHaveBeenCalledTimes(1); + expect(superSpy).toHaveBeenCalledWith(mockNodes[7].superBlock); + }); + + it('should use the currentChallengeId prop if it exists', () => { + const props = { ...baseProps, currentChallengeId }; + const mapToRender = ; + shallow(mapToRender); + + expect(initializeSpy).toHaveBeenCalledTimes(1); + expect(initializeSpy).toHaveBeenCalledWith(currentChallengeId); + }); + + it('should default to the first challenge otherwise', () => { + const blockSpy = jest.fn(); + const superSpy = jest.fn(); + const props = { + ...baseProps, + toggleBlock: blockSpy, + toggleSuperBlock: superSpy + }; + const mapToRender = ; + shallow(mapToRender); + expect(blockSpy).toHaveBeenCalledTimes(1); + expect(blockSpy).toHaveBeenCalledWith(mockNodes[0].block); + + expect(superSpy).toHaveBeenCalledTimes(1); + expect(superSpy).toHaveBeenCalledWith(mockNodes[0].superBlock); + }); + + it('calls resetExpansion when initialising', () => { + const expansionSpy = jest.fn(); + const props = { + ...baseProps, + resetExpansion: expansionSpy + }; + const mapToRender = ; + shallow(mapToRender); + expect(expansionSpy).toHaveBeenCalledTimes(1); + }); + }); +}); diff --git a/client/src/components/Map/components/Block.js b/client/src/components/Map/components/Block.js index 9a9bc3fed6..fdea2b1933 100644 --- a/client/src/components/Map/components/Block.js +++ b/client/src/components/Map/components/Block.js @@ -7,7 +7,7 @@ import { Link } from 'gatsby'; import ga from '../../../analytics'; import { makeExpandedBlockSelector, toggleBlock } from '../redux'; -import { userSelector } from '../../../redux'; +import { completedChallengesSelector } from '../../../redux'; import Caret from '../../../assets/icons/Caret'; import { blockNameify } from '../../../../utils/blockNameify'; /* eslint-disable max-len */ @@ -19,8 +19,8 @@ const mapStateToProps = (state, ownProps) => { return createSelector( expandedSelector, - userSelector, - (isExpanded, { completedChallenges = [] }) => ({ + completedChallengesSelector, + (isExpanded, completedChallenges) => ({ isExpanded, completedChallenges: completedChallenges.map(({ id }) => id) }) @@ -127,6 +127,7 @@ export class Block extends Component { } return { ...challenge, isCompleted }; }); + return (