Merge pull request #43684 from code-dot-org/lesson-edit-hide-button

Refactor SaveBar with hide Show button

apps/src/lib/levelbuilder/SaveBar.jsx

@@ -1,71 +1,87 @@
 import PropTypes from 'prop-types';
-import React, {Component} from 'react';
+import React from 'react';
 import color from '@cdo/apps/util/color';
 import FontAwesome from '@cdo/apps/templates/FontAwesome';
+import {navigateToHref} from '@cdo/apps/utils';
 
-export default class SaveBar extends Component {
-  static propTypes = {
-    lastSaved: PropTypes.number,
-    error: PropTypes.string,
-    handleSave: PropTypes.func.isRequired,
-    handleView: PropTypes.func.isRequired,
-    isSaving: PropTypes.bool
+export default function SaveBar({
+  error,
+  handleSave,
+  handleView,
+  isSaving,
+  lastSaved,
+  pathForShowButton
+}) {
+  const renderShowButtonOrPlaceholder = () => {
+    const isShowButtonVisible = handleView || pathForShowButton;
+
+    const renderShowButton = () => (
+      <button
+        className="btn"
+        type="button"
+        style={styles.saveButton}
+        onClick={handleView || (() => navigateToHref(pathForShowButton))}
+        disabled={isSaving}
+      >
+        Show
+      </button>
+    );
+
+    return isShowButtonVisible ? renderShowButton() : <span />;
   };
 
-  render() {
-    return (
-      <div style={styles.saveButtonBackground} className="saveBar">
+  return (
+    <div style={styles.saveButtonBackground} className="saveBar">
+      {renderShowButtonOrPlaceholder()}
+      <div style={styles.saveButtonArea}>
+        {lastSaved && !error && (
+          <div style={styles.lastSaved} className="lastSavedMessage">
+            {`Last saved at: ${new Date(lastSaved).toLocaleString()}`}
+          </div>
+        )}
+        {error && <div style={styles.error}>{`Error Saving: ${error}`}</div>}
+        {isSaving && (
+          <div style={styles.spinner}>
+            <FontAwesome icon="spinner" className="fa-spin" />
+          </div>
+        )}
         <button
           className="btn"
           type="button"
           style={styles.saveButton}
-          onClick={this.props.handleView}
-          disabled={this.props.isSaving}
+          onClick={e => handleSave(e, false)}
+          disabled={isSaving}
+        >
+          Save and Keep Editing
+        </button>
+        <button
+          className="btn btn-primary"
+          type="submit"
+          style={styles.saveButton}
+          onClick={e => handleSave(e, true)}
+          disabled={isSaving}
         >
-          Show
+          Save and Close
         </button>
-        <div style={styles.saveButtonArea}>
-          {this.props.lastSaved && !this.props.error && (
-            <div style={styles.lastSaved} className="lastSavedMessage">
-              {`Last saved at: ${new Date(
-                this.props.lastSaved
-              ).toLocaleString()}`}
-            </div>
-          )}
-          {this.props.error && (
-            <div style={styles.error}>{`Error Saving: ${
-              this.props.error
-            }`}</div>
-          )}
-          {this.props.isSaving && (
-            <div style={styles.spinner}>
-              <FontAwesome icon="spinner" className="fa-spin" />
-            </div>
-          )}
-          <button
-            className="btn"
-            type="button"
-            style={styles.saveButton}
-            onClick={e => this.props.handleSave(e, false)}
-            disabled={this.props.isSaving}
-          >
-            Save and Keep Editing
-          </button>
-          <button
-            className="btn btn-primary"
-            type="submit"
-            style={styles.saveButton}
-            onClick={e => this.props.handleSave(e, true)}
-            disabled={this.props.isSaving}
-          >
-            Save and Close
-          </button>
-        </div>
       </div>
-    );
-  }
+    </div>
+  );
 }
 
+SaveBar.propTypes = {
+  error: PropTypes.string,
+  handleSave: PropTypes.func.isRequired,
+  handleView: PropTypes.func,
+  isSaving: PropTypes.bool,
+  lastSaved: PropTypes.number,
+  pathForShowButton: PropTypes.string
+};
+
+SaveBar.defaultProps = {
+  isSaving: false,
+  lastSaved: 0
+};
+
 const styles = {
   saveButtonBackground: {
     margin: 0,

apps/src/lib/levelbuilder/course-editor/CourseEditor.js

@@ -104,10 +104,6 @@ class CourseEditor extends Component {
     this.setState({announcements: newAnnouncements});
   };
 
-  handleView = () => {
-    navigateToHref(linkWithQueryParams(this.props.coursePath));
-  };
-
   handleSave = (event, shouldCloseAfterSave) => {
     event.preventDefault();
 
@@ -396,10 +392,10 @@ class CourseEditor extends Component {
         </CollapsibleEditorSection>
         <SaveBar
           handleSave={this.handleSave}
-          handleView={this.handleView}
           error={this.state.error}
           isSaving={this.state.isSaving}
           lastSaved={this.state.lastSaved}
+          pathForShowButton={this.props.coursePath}
         />
       </div>
     );

apps/src/lib/levelbuilder/lesson-editor/LessonEditor.jsx

@@ -62,8 +62,8 @@ class LessonEditor extends Component {
       unplugged: this.props.initialLessonData.unplugged,
       lockable: this.props.initialLessonData.lockable,
       hasLessonPlan: this.props.initialLessonData.hasLessonPlan,
-      creativeCommonsLicense: this.props.initialLessonData
-        .creativeCommonsLicense,
+      creativeCommonsLicense:
+        this.props.initialLessonData.creativeCommonsLicense || '',
       assessment: this.props.initialLessonData.assessment,
       purpose: this.props.initialLessonData.purpose || '',
       preparation: this.props.initialLessonData.preparation || '',
@@ -73,12 +73,6 @@ class LessonEditor extends Component {
     };
   }
 
-  handleView = () => {
-    navigateToHref(
-      linkWithQueryParams(this.state.originalLessonData.lessonPath)
-    );
-  };
-
   handleSave = (event, shouldCloseAfterSave) => {
     event.preventDefault();
 
@@ -457,10 +451,14 @@ class LessonEditor extends Component {
 
         <SaveBar
           handleSave={this.handleSave}
-          handleView={this.handleView}
           error={this.state.error}
           isSaving={this.state.isSaving}
           lastSaved={this.state.lastSaved}
+          pathForShowButton={
+            this.props.initialLessonData.hasLessonPlan
+              ? this.state.originalLessonData.lessonPath
+              : undefined
+          }
         />
       </div>
     );

apps/src/lib/levelbuilder/unit-editor/UnitEditor.jsx

@@ -216,10 +216,6 @@ class UnitEditor extends React.Component {
     }
   };
 
-  handleView = () => {
-    navigateToHref(linkWithQueryParams(this.props.scriptPath));
-  };
-
   handleSave = (event, shouldCloseAfterSave) => {
     event.preventDefault();
 
@@ -1102,10 +1098,10 @@ class UnitEditor extends React.Component {
         </CollapsibleEditorSection>
         <SaveBar
           handleSave={this.handleSave}
-          handleView={this.handleView}
           error={this.state.error}
           isSaving={this.state.isSaving}
           lastSaved={this.state.lastSaved}
+          pathForShowButton={this.props.scriptPath}
         />
       </div>
     );

apps/test/unit/lib/levelbuilder/SaveBarTest.jsx

@@ -3,31 +3,25 @@ import {shallow} from 'enzyme';
 import {expect} from '../../../util/reconfiguredChai';
 import sinon from 'sinon';
 import SaveBar from '@cdo/apps/lib/levelbuilder/SaveBar';
+import * as utils from '@cdo/apps/utils';
 
 describe('SaveBar', () => {
-  let defaultProps, handleSave, handleView;
+  let handleSave;
   beforeEach(() => {
     handleSave = sinon.spy();
-    handleView = sinon.spy();
-    defaultProps = {
-      isSaving: false,
-      error: null,
-      lastSaved: null,
-      handleSave,
-      handleView
-    };
   });
 
   it('renders default props', () => {
-    const wrapper = shallow(<SaveBar {...defaultProps} />);
-    expect(wrapper.find('button').length).to.equal(3);
+    const wrapper = shallow(<SaveBar handleSave={handleSave} />);
+    expect(wrapper.find('button').length).to.equal(2); // show button not rendered
     expect(wrapper.find('FontAwesome').length).to.equal(0); //spinner isn't showing
   });
 
   it('can save and keep editing', () => {
-    const wrapper = shallow(<SaveBar {...defaultProps} />);
+    const handleSave = sinon.spy();
+    const wrapper = shallow(<SaveBar handleSave={handleSave} />);
 
-    const saveAndKeepEditingButton = wrapper.find('button').at(1);
+    const saveAndKeepEditingButton = wrapper.find('button').at(0);
     expect(saveAndKeepEditingButton.contains('Save and Keep Editing')).to.be
       .true;
     saveAndKeepEditingButton.simulate('click');
@@ -36,15 +30,17 @@ describe('SaveBar', () => {
   });
 
   it('shows spinner when isSaving is true', () => {
-    const wrapper = shallow(<SaveBar {...defaultProps} isSaving={true} />);
+    const wrapper = shallow(
+      <SaveBar handleSave={handleSave} isSaving={true} />
+    );
 
     // check the the spinner is showing
     expect(wrapper.find('FontAwesome').length).to.equal(1);
   });
 
   it('shows lastSaved when there is no error', () => {
     const wrapper = shallow(
-      <SaveBar {...defaultProps} lastSaved={Date.now()} />
+      <SaveBar handleSave={handleSave} lastSaved={Date.now()} />
     );
 
     expect(wrapper.find('.lastSavedMessage').text()).to.include(
@@ -54,7 +50,7 @@ describe('SaveBar', () => {
 
   it('shows error when props error is set', () => {
     const wrapper = shallow(
-      <SaveBar {...defaultProps} error={'There was an error'} />
+      <SaveBar handleSave={handleSave} error={'There was an error'} />
     );
     expect(wrapper.find('.saveBar').find('FontAwesome').length).to.equal(0);
     expect(
@@ -63,22 +59,50 @@ describe('SaveBar', () => {
   });
 
   it('can save and close', () => {
-    const wrapper = shallow(<SaveBar {...defaultProps} />);
+    const handleSave = sinon.spy();
+    const wrapper = shallow(<SaveBar handleSave={handleSave} />);
 
-    const saveAndCloseButton = wrapper.find('button').at(2);
+    const saveAndCloseButton = wrapper.find('button').at(1);
     expect(saveAndCloseButton.contains('Save and Close')).to.be.true;
     saveAndCloseButton.simulate('click');
 
     expect(handleSave).to.have.been.calledOnce;
   });
 
-  it('can go to item with show', () => {
-    const wrapper = shallow(<SaveBar {...defaultProps} />);
+  it('can show with custom handleView, even if path is given', () => {
+    const handleView = sinon.spy();
+    sinon.stub(utils, 'navigateToHref');
+    const wrapper = shallow(
+      <SaveBar
+        handleSave={handleSave}
+        handleView={handleView}
+        pathForShowButton={'/my/path'}
+      />
+    );
 
-    const saveAndCloseButton = wrapper.find('button').at(0);
-    expect(saveAndCloseButton.contains('Show')).to.be.true;
-    saveAndCloseButton.simulate('click');
+    const showButton = wrapper.find('button').at(0);
+    expect(showButton.contains('Show')).to.be.true;
+    showButton.simulate('click');
 
+    expect(utils.navigateToHref).not.to.have.been.called;
     expect(handleView).to.have.been.calledOnce;
+
+    utils.navigateToHref.restore();
+  });
+
+  it('can show with custom path', () => {
+    const path = '/my/path';
+    sinon.stub(utils, 'navigateToHref');
+    const wrapper = shallow(
+      <SaveBar handleSave={handleSave} pathForShowButton={path} />
+    );
+
+    const showButton = wrapper.find('button').at(0);
+    expect(showButton.contains('Show')).to.be.true;
+    showButton.simulate('click');
+
+    expect(utils.navigateToHref).to.have.been.calledWith(path);
+
+    utils.navigateToHref.restore();
   });
 });