Skip to content

Commit

Permalink
Bug fixes and optimizations (#173)
Browse files Browse the repository at this point in the history
* Fixing marginal bugs in exercise simple edit form.

* Optimizing performance of edit exercise form.

* Prevent unnecessary sidebar re-rendering.

* Final step in edit exercise form performance optimizations.

* Forgotten bug.

* Additional performance tweaks on EditAssignment form.
  • Loading branch information
Martin Kruliš committed Feb 11, 2018
1 parent 5e33f82 commit 85120fd
Show file tree
Hide file tree
Showing 19 changed files with 264 additions and 209 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ const ExercisesListItem = ({
createdAt,
isLocked,
isBroken,
validationError,
createActions,
locale,
links: { EXERCISE_URI_FACTORY }
Expand All @@ -31,12 +30,7 @@ const ExercisesListItem = ({
<Icon name="code" />
</td>
<td>
<ExercisePrefixIcons
id={id}
isLocked={isLocked}
isBroken={isBroken}
validationError={validationError}
/>
<ExercisePrefixIcons id={id} isLocked={isLocked} isBroken={isBroken} />
<strong>
<Link to={EXERCISE_URI_FACTORY(id)}>
<LocalizedExerciseName entity={{ name, localizedTexts }} />
Expand Down Expand Up @@ -82,7 +76,6 @@ ExercisesListItem.propTypes = {
createdAt: PropTypes.number.isRequired,
isLocked: PropTypes.bool.isRequired,
isBroken: PropTypes.bool.isRequired,
validationError: PropTypes.string,
localizedTexts: PropTypes.array.isRequired,
createActions: PropTypes.func,
locale: PropTypes.string.isRequired,
Expand Down
9 changes: 1 addition & 8 deletions src/components/Exercises/ExercisesName/ExercisesName.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,11 @@ const ExercisesName = ({
localizedTexts,
isLocked,
isBroken,
validationError,
noLink,
links: { EXERCISE_URI_FACTORY }
}) =>
<span>
<ExercisePrefixIcons
id={id}
isLocked={isLocked}
isBroken={isBroken}
validationError={validationError}
/>
<ExercisePrefixIcons id={id} isLocked={isLocked} isBroken={isBroken} />
{noLink
? <span>
<LocalizedExerciseName entity={{ name, localizedTexts }} />
Expand All @@ -39,7 +33,6 @@ ExercisesName.propTypes = {
localizedTexts: PropTypes.array.isRequired,
isLocked: PropTypes.bool.isRequired,
isBroken: PropTypes.bool.isRequired,
validationError: PropTypes.string,
noLink: PropTypes.bool,
links: PropTypes.object
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,14 @@ const ExercisesSimpleListItem = ({
authorId,
isLocked,
isBroken,
validationError,
localizedTexts,
createActions,
locale,
links: { EXERCISE_URI_FACTORY }
}) =>
<tr>
<td>
<ExercisePrefixIcons
id={id}
isLocked={isLocked}
isBroken={isBroken}
validationError={validationError}
/>
<ExercisePrefixIcons id={id} isLocked={isLocked} isBroken={isBroken} />
<strong>
<Link to={EXERCISE_URI_FACTORY(id)}>
<LocalizedExerciseName entity={{ name, localizedTexts }} />
Expand All @@ -54,7 +48,6 @@ ExercisesSimpleListItem.propTypes = {
difficulty: PropTypes.string.isRequired,
isLocked: PropTypes.bool.isRequired,
isBroken: PropTypes.bool.isRequired,
validationError: PropTypes.string,
localizedTexts: PropTypes.array.isRequired,
createActions: PropTypes.func,
locale: PropTypes.string.isRequired,
Expand Down
21 changes: 10 additions & 11 deletions src/components/forms/EditAssignmentForm/EditAssignmentForm.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ const EditAssignmentForm = ({
submitSucceeded: hasSucceeded,
asyncValidating,
invalid,
formValues: { firstDeadline, allowSecondDeadline, localizedTexts } = {}
firstDeadline,
allowSecondDeadline,
localizedTextsLocales
}) =>
<div>
<FormBox
Expand Down Expand Up @@ -81,7 +83,7 @@ const EditAssignmentForm = ({

<FieldArray
name="localizedTexts"
localizedTexts={localizedTexts}
localizedTextsLocales={localizedTextsLocales}
component={LocalizedTextsFormField}
/>

Expand Down Expand Up @@ -225,14 +227,9 @@ EditAssignmentForm.propTypes = {
submitSucceeded: PropTypes.bool,
asyncValidating: PropTypes.oneOfType([PropTypes.bool, PropTypes.string]),
invalid: PropTypes.bool,
formValues: PropTypes.shape({
firstDeadline: PropTypes.oneOfType([PropTypes.number, PropTypes.object]), // object == moment.js instance
allowSecondDeadline: PropTypes.oneOfType([
PropTypes.bool,
PropTypes.string
]),
localizedTexts: PropTypes.array
})
firstDeadline: PropTypes.oneOfType([PropTypes.number, PropTypes.object]), // object == moment.js instance
allowSecondDeadline: PropTypes.oneOfType([PropTypes.bool, PropTypes.string]),
localizedTextsLocales: PropTypes.array
};

const isNonNegativeInteger = n =>
Expand Down Expand Up @@ -442,5 +439,7 @@ const asyncValidate = (values, dispatch, { assignment: { id, version } }) =>
export default reduxForm({
form: 'editAssignment',
validate,
asyncValidate
asyncValidate,
enableReinitialize: true,
keepDirtyOnReinitialize: false
})(EditAssignmentForm);
61 changes: 42 additions & 19 deletions src/components/forms/EditExerciseForm/EditExerciseForm.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
import { Alert } from 'react-bootstrap';
import Icon from 'react-fontawesome';
import { LinkContainer } from 'react-router-bootstrap';
import { defaultMemoize } from 'reselect';

import { SelectField, CheckboxField } from '../Fields';

Expand All @@ -36,6 +37,12 @@ const messages = defineMessages({
}
});

const difficultyOptions = defaultMemoize(formatMessage => [
{ key: 'easy', name: formatMessage(messages.easy) },
{ key: 'medium', name: formatMessage(messages.medium) },
{ key: 'hard', name: formatMessage(messages.hard) }
]);

const EditExerciseForm = ({
initialValues: exercise,
anyTouched,
Expand All @@ -45,8 +52,8 @@ const EditExerciseForm = ({
submitSucceeded: hasSucceeded,
invalid,
asyncValidating,
formValues: { localizedTexts } = {},
intl: { formatMessage, locale },
localizedTextsLocales = [],
intl: { formatMessage },
links: { EXERCISE_EDIT_SIMPLE_CONFIG_URI_FACTORY }
}) =>
<FormBox
Expand Down Expand Up @@ -121,19 +128,15 @@ const EditExerciseForm = ({

<FieldArray
name="localizedTexts"
localizedTexts={localizedTexts}
localizedTextsLocales={localizedTextsLocales}
component={LocalizedTextsFormField}
/>

<Field
name="difficulty"
component={SelectField}
options={[
{ key: '', name: '...' },
{ key: 'easy', name: formatMessage(messages.easy) },
{ key: 'medium', name: formatMessage(messages.medium) },
{ key: 'hard', name: formatMessage(messages.hard) }
]}
options={difficultyOptions(formatMessage)}
addEmptyOption={true}
label={
<FormattedMessage
id="app.editExerciseForm.difficulty"
Expand Down Expand Up @@ -178,9 +181,7 @@ EditExerciseForm.propTypes = {
submitSucceeded: PropTypes.bool,
invalid: PropTypes.bool,
asyncValidating: PropTypes.oneOfType([PropTypes.bool, PropTypes.string]),
formValues: PropTypes.shape({
localizedTexts: PropTypes.array
}),
localizedTextsLocales: PropTypes.array,
links: PropTypes.object
};

Expand Down Expand Up @@ -300,12 +301,34 @@ const asyncValidate = (values, dispatch, { initialValues: { id, version } }) =>
.catch(errors => reject(errors))
);

// Actually, this is reimplementation of default behavior from documentation.
// For some reason, the asyncValidation is by default called also for every change event (which is not documented).
const shouldAsyncValidate = ({
syncValidationPasses,
trigger,
pristine,
initialized
}) => {
if (!syncValidationPasses) {
return false;
}
switch (trigger) {
case 'blur':
return true;
case 'submit':
return !pristine || !initialized;
default:
return false;
}
};

export default withLinks(
injectIntl(
reduxForm({
form: 'editExercise',
validate,
asyncValidate
})(EditExerciseForm)
)
reduxForm({
form: 'editExercise',
validate,
asyncValidate,
shouldAsyncValidate,
enableReinitialize: true,
keepDirtyOnReinitialize: false
})(injectIntl(EditExerciseForm))
);
10 changes: 5 additions & 5 deletions src/components/forms/EditGroupForm/EditGroupForm.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ const EditGroupForm = ({
submitSucceeded = false,
invalid,
createNew = false,
formValues: { localizedTexts, hasThreshold } = {},
localizedTextsLocales,
hasThreshold,
collapsable = false,
isOpen = true,
reset
Expand Down Expand Up @@ -85,7 +86,7 @@ const EditGroupForm = ({

<FieldArray
name="localizedTexts"
localizedTexts={localizedTexts}
localizedTextsLocales={localizedTextsLocales}
component={LocalizedTextsFormField}
isGroup={true}
/>
Expand Down Expand Up @@ -176,9 +177,8 @@ EditGroupForm.propTypes = {
submitSucceeded: PropTypes.bool,
submitting: PropTypes.bool,
invalid: PropTypes.bool,
formValues: PropTypes.shape({
localizedTexts: PropTypes.array
}),
hasThreshold: PropTypes.bool,
localizedTextsLocales: PropTypes.array,
createNew: PropTypes.bool,
collapsable: PropTypes.bool,
isOpen: PropTypes.bool,
Expand Down
2 changes: 1 addition & 1 deletion src/components/forms/EditSimpleLimitsForm/styles.less
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,6 @@
color: #666;
}

.preciseTimeTooltip:first-child {
.preciseTimeTooltip :first-child {
margin-right: 0.5em;
}
22 changes: 8 additions & 14 deletions src/components/forms/Fields/LanguageSelectField.js
Original file line number Diff line number Diff line change
@@ -1,20 +1,14 @@
import React from 'react';
import PropTypes from 'prop-types';
import SelectField from './SelectField';

const LanguageSelectField = ({ options = [], ...props }) =>
<SelectField
{...props}
options={[
{ key: '', name: '...' },
{ key: 'en', name: 'English' },
{ key: 'cs', name: 'Čeština' },
...options
]}
/>;
const languageOptions = [
{ key: 'en', name: 'English' },
{ key: 'cs', name: 'Čeština' }
];

LanguageSelectField.propTypes = {
options: PropTypes.array
};
const LanguageSelectField = ({ ...props }) =>
<SelectField {...props} options={languageOptions} addEmptyOption={true} />;

LanguageSelectField.propTypes = {};

export default LanguageSelectField;
7 changes: 3 additions & 4 deletions src/components/forms/Fields/TabbedArrayField.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ class TabbedArrayField extends Component {
activeKey={this.state.activeTab}
onSelect={this.changeTab}
>
{fields.map((prefix, i) => (
{fields.map((prefix, i) =>
<Tab key={i} eventKey={i} title={getTitle(i)}>
<ContentComponent {...props} i={i} prefix={prefix} />
{remove &&
Expand All @@ -73,8 +73,7 @@ class TabbedArrayField extends Component {
}}
>
<Button bsStyle="default">
<WarningIcon />
{' '}
<WarningIcon />{' '}
<FormattedMessage
id="app.tabbedArrayField.remove"
defaultMessage="Remove"
Expand All @@ -83,7 +82,7 @@ class TabbedArrayField extends Component {
</Confirm>
</p>}
</Tab>
))}
)}
</Tabs>}

{fields.length === 0 &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,15 @@ import LocalizedExerciseFormField from './LocalizedExerciseFormField';
import LocalizedGroupFormField from './LocalizedGroupFormField';

const LocalizedTextsFormField = ({
localizedTexts = [],
localizedTextsLocales = [],
isGroup = false,
...props
}) =>
<TabbedArrayField
{...props}
getTitle={i =>
localizedTexts && localizedTexts[i] && localizedTexts[i].locale
? localizedTexts[i].locale
localizedTextsLocales && localizedTextsLocales[i]
? localizedTextsLocales[i]
: <FormattedMessage
id="app.editLocalizedTextForm.newLocale"
defaultMessage="New language"
Expand Down Expand Up @@ -46,7 +46,7 @@ const LocalizedTextsFormField = ({
/>;

LocalizedTextsFormField.propTypes = {
localizedTexts: PropTypes.array,
localizedTextsLocales: PropTypes.array,
isGroup: PropTypes.bool
};

Expand Down
Loading

0 comments on commit 85120fd

Please sign in to comment.