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

Add a WYSIWYG-esque editor for instructions #1426

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -194,13 +194,15 @@
"qs": "^6.1.0",
"react": "^16.3.2",
"react-copy-to-clipboard": "^5.0.0",
"react-coroutine": "^2.0.2",
"react-dom": "^16.0.0",
"react-draggable": "^3.0.3",
"react-ga": "^2.5.0",
"react-immutable-proptypes": "^2.1.0",
"react-onclickoutside": "^6.5.0",
"react-prevent-clickthrough": "^0.0.3",
"react-redux": "^5.0.3",
"react-simplemde-editor": "^3.6.14",
"reduce-reducers": "^0.1.2",
"redux": "^3.6.0",
"redux-actions": "^2.2.1",
Expand Down
2 changes: 2 additions & 0 deletions src/actions/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import {
toggleTopBarMenu,
closeTopBarMenu,
startEditingInstructions,
continueEditingInstructions,
cancelEditingInstructions,
} from './ui';

Expand Down Expand Up @@ -97,6 +98,7 @@ export {
toggleTopBarMenu,
closeTopBarMenu,
startEditingInstructions,
continueEditingInstructions,
cancelEditingInstructions,
logIn,
logOut,
Expand Down
5 changes: 5 additions & 0 deletions src/actions/ui.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,11 @@ export const startEditingInstructions = createAction(
(_projectKey, timestamp = Date.now()) => ({timestamp}),
);

export const continueEditingInstructions = createAction(
Copy link
Contributor

Choose a reason for hiding this comment

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

I wasn’t sure what continueEditingInstructions was based on the name (had to look at where it was used)—maybe we could do something more explicit/verbose like updateInProgressInstructions?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is of course the camel case version of the action name, and for the action I was following the pattern of the actions {verb}_EDITING_INSTRUCTIONS. Other verbs are start and cancel. You would rather break that pattern? It just seems a bit less intuitive but I'm fine with it. So it would be UPDATE_IN_PROGRESS_INSTRUCTIONS? I figure continueEditingInstructions fits in nicely with startEditingInstructions and cancelEditingInstructions. Another idea for something more verbose is continueEditingUnsavedInstructions.

Copy link
Contributor

Choose a reason for hiding this comment

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

Definitely understand the appeal of the symmetry, but ultimately I think clarity/expressiveness is the overriding concern here. Given that you went with draftInstructions for the property name (which I like) I would propose updateDraftInstructions here. WDYT?

'CONTINUE_EDITING_INSTRUCTIONS',
(projectKey, content) => ({projectKey, content}),
);

export const cancelEditingInstructions = createAction(
'CANCEL_EDITING_INSTRUCTIONS',
);
5 changes: 4 additions & 1 deletion src/components/Instructions.jsx
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
import React from 'react';
import PropTypes from 'prop-types';
import {toReact as markdownToReact} from '../util/markdown';
import InstructionsEditor from './InstructionsEditor';
import InstructionsEditor from './InstructionsEditorAsync';

export default function Instructions({
instructions,
isEditing,
isOpen,
projectKey,
onCancelEditing,
onContinueEditing,
onSaveChanges,
}) {
if (!isEditing && !instructions || !isOpen) {
Expand All @@ -25,6 +26,7 @@ export default function Instructions({
instructions={instructions}
projectKey={projectKey}
onCancelEditing={onCancelEditing}
onContinueEditing={onContinueEditing}
onSaveChanges={onSaveChanges}
/> :
<div className="instructions">
Expand All @@ -41,5 +43,6 @@ Instructions.propTypes = {
isOpen: PropTypes.bool.isRequired,
projectKey: PropTypes.string.isRequired,
onCancelEditing: PropTypes.func.isRequired,
onContinueEditing: PropTypes.func.isRequired,
onSaveChanges: PropTypes.func.isRequired,
};
37 changes: 20 additions & 17 deletions src/components/InstructionsEditor.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,29 +3,29 @@ import PropTypes from 'prop-types';
import {t} from 'i18next';
import bindAll from 'lodash/bindAll';

import SimpleMDE from 'react-simplemde-editor';

export default class InstructionsEditor extends React.Component {
constructor() {
super();
bindAll(this, '_handleCancelEditing', '_handleSaveChanges', '_ref');
}

componentDidMount() {
if (!this.props.instructions) {
this._editor.focus();
}
bindAll(
this,
'_handleCancelEditing',
'_handleContinueEditing',
'_handleSaveChanges',
);
}

_handleCancelEditing() {
this.props.onCancelEditing();
}

_handleSaveChanges() {
const newValue = this._editor.value.trim();
this.props.onSaveChanges(this.props.projectKey, newValue);
this.props.onSaveChanges(this.props.projectKey, this.props.instructions);
}

_ref(editorElement) {
this._editor = editorElement;
_handleContinueEditing(newValue) {
this.props.onContinueEditing(this.props.projectKey, newValue);
}

render() {
Expand All @@ -46,11 +46,13 @@ export default class InstructionsEditor extends React.Component {
</button>
</div>
<div className="instructions-editor__input-container">
<textarea
className="instructions-editor__input"
defaultValue={this.props.instructions}
placeholder="Type here..."
ref={this._ref}
<SimpleMDE
options={{
autofocus: true,
spellChecker: false,
}}
value={this.props.instructions}
onChange={this._handleContinueEditing}
/>
</div>
<div className="instructions-editor__footer">
Expand All @@ -60,7 +62,7 @@ export default class InstructionsEditor extends React.Component {
rel="noopener noreferrer"
target="_blank"
>
Styling with Markdown is supported
Styling with Markdown is supported
</a>
</div>
</div>
Expand All @@ -72,5 +74,6 @@ InstructionsEditor.propTypes = {
instructions: PropTypes.string.isRequired,
projectKey: PropTypes.string.isRequired,
onCancelEditing: PropTypes.func.isRequired,
onContinueEditing: PropTypes.func.isRequired,
onSaveChanges: PropTypes.func.isRequired,
};
24 changes: 24 additions & 0 deletions src/components/InstructionsEditorAsync.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import React from 'react';
import Coroutine from 'react-coroutine';

async function* InstructionsEditorAsync({
instructions,
projectKey,
onCancelEditing,
onContinueEditing,
onSaveChanges,
}) {
yield <span />;
const {'default': InstructionsEditor} = await import('./InstructionsEditor');
return (
<InstructionsEditor
instructions={instructions}
projectKey={projectKey}
onCancelEditing={onCancelEditing}
onContinueEditing={onContinueEditing}
onSaveChanges={onSaveChanges}
/>
);
}

export default Coroutine.create(InstructionsEditorAsync);
4 changes: 4 additions & 0 deletions src/containers/Instructions.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
} from '../selectors';
import {
cancelEditingInstructions,
continueEditingInstructions,
updateProjectInstructions,
} from '../actions';

Expand All @@ -25,6 +26,9 @@ function mapDispatchToProps(dispatch) {
onCancelEditing() {
dispatch(cancelEditingInstructions());
},
onContinueEditing(projectKey, newValue) {
dispatch(continueEditingInstructions(projectKey, newValue));
},
onSaveChanges(projectKey, newValue) {
dispatch(updateProjectInstructions(projectKey, newValue));
},
Expand Down
3 changes: 2 additions & 1 deletion src/css/application.css
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ body {
.layout__instructions {
flex: 0 0 25%;
position: relative;
animation: 0.2s ease-in 0.1s both fadeIn;
}

/** @define top-bar */
Expand Down Expand Up @@ -388,7 +389,7 @@ body {
.instructions-editor__menu {
padding: 0.5rem;
text-align: right;
background-color: var(--color-light-gray);
background-color: var(--color-low-contrast-gray);
}

.instructions-editor__footer {
Expand Down
Loading