Skip to content

Commit

Permalink
Merge pull request #117 from Tietokilta/fix/editor-perf
Browse files Browse the repository at this point in the history
Rewrite editor using react-final-form
  • Loading branch information
PurkkaKoodari authored Jan 18, 2024
2 parents 4089166 + ee909aa commit 60d8a42
Show file tree
Hide file tree
Showing 31 changed files with 1,609 additions and 1,154 deletions.
21 changes: 12 additions & 9 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,20 +25,20 @@
"url": "https://github.com/Tietokilta/ilmomasiina/issues"
},
"dependencies": {
"@typescript-eslint/eslint-plugin": "^5.48.2",
"@typescript-eslint/parser": "^5.48.2",
"eslint": "^8.32.0",
"@typescript-eslint/eslint-plugin": "^6.7.3",
"@typescript-eslint/parser": "^6.7.3",
"eslint": "^8.50.0",
"eslint-config-airbnb": "^19.0.4",
"eslint-config-airbnb-typescript": "^17.0.0",
"eslint-plugin-import": "^2.27.5",
"eslint-plugin-jest": "^26.9.0",
"eslint-config-airbnb-typescript": "^17.1.0",
"eslint-plugin-import": "^2.28.1",
"eslint-plugin-jest": "^27.4.0",
"eslint-plugin-jsx-a11y": "^6.7.1",
"eslint-plugin-promise": "^6.1.1",
"eslint-plugin-react": "^7.32.1",
"eslint-plugin-react": "^7.33.2",
"eslint-plugin-react-hooks": "^4.6.0",
"eslint-plugin-simple-import-sort": "^7.0.0",
"eslint-plugin-simple-import-sort": "^10.0.0",
"pnpm": "^7.25.0",
"typescript": "~4.9"
"typescript": "~5.2.2"
},
"browserslist": {
"production": [
Expand All @@ -64,6 +64,9 @@
"popper.js",
"eslint"
]
},
"overrides": {
"@types/react": "^17.0.47"
}
}
}
10 changes: 6 additions & 4 deletions packages/ilmomasiina-components/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,19 +29,21 @@
"@types/lodash": "^4.14.182",
"@types/react": "^17 || ^18",
"bootstrap": "^4.6.1",
"formik": "^2.2.9",
"final-form": "^4.20.10",
"formik": "^2.4.5",
"i18next": "^22.4.11",
"lodash": "^4.17.21",
"moment": "^2.29.3",
"moment-timezone": "^0.5.34",
"react": "^17 || ^18",
"react-bootstrap": "^1.6.5",
"react-countdown-now": "^2.1.2",
"react-dom": "^17 || ^18",
"react-final-form": "^6.5.9",
"react-i18next": "^12.2.0",
"react-markdown": "^8.0.3",
"react-toastify": "^8.2.0",
"remark-gfm": "^3.0.1",
"i18next": "^22.4.11",
"react-i18next": "^12.2.0"
"remark-gfm": "^3.0.1"
},
"devDependencies": {
"rimraf": "^3.0.2",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import React, { ReactNode } from 'react';

import { Col, Form, Row } from 'react-bootstrap';

export type BaseFieldRowProps = {
/** The name of the field in the Formik data. */
name: string;
/** The label placed in the left column. */
label?: string;
/** The help string placed below the field. */
help?: ReactNode;
/** Whether the field is required. */
required?: boolean;
/** Error message rendered below the field. */
error?: ReactNode;
/** Extra feedback rendered below the field. Bring your own `Form.Control.Feedback`. */
extraFeedback?: ReactNode;
/** `true` to adjust the vertical alignment of the left column label for checkboxes/radios. */
checkAlign?: boolean;
children: ReactNode;
};

export default function BaseFieldRow({
name,
label = '',
help,
required = false,
extraFeedback,
checkAlign,
children,
error,
}: BaseFieldRowProps) {
return (
<Form.Group as={Row} controlId={name}>
<Form.Label column sm="3" data-required={required} className={checkAlign ? 'pt-0' : ''}>{label}</Form.Label>
<Col sm="9">
{children}
{error && (
<Form.Control.Feedback type="invalid">
{error}
</Form.Control.Feedback>
)}
{extraFeedback}
{help && <Form.Text muted>{help}</Form.Text>}
</Col>
</Form.Group>
);
}
52 changes: 20 additions & 32 deletions packages/ilmomasiina-components/src/components/FieldRow/index.tsx
Original file line number Diff line number Diff line change
@@ -1,23 +1,13 @@
import React, { ComponentType, ReactNode } from 'react';

import { Field, useFormikContext } from 'formik';
import { Col, Form, Row } from 'react-bootstrap';
import { Field, useField } from 'formik';
import { Form } from 'react-bootstrap';

type Props = {
/** The name of the field in the Formik data. */
name: string;
/** The label placed in the left column. */
label?: string;
/** The help string placed below the field. */
help?: ReactNode;
/** Whether the field is required. */
required?: boolean;
import BaseFieldRow, { BaseFieldRowProps } from '../BaseFieldRow';

type Props = Omit<BaseFieldRowProps, 'error' | 'children'> & {
/** Overrides the real error message if the field has errors. */
alternateError?: string;
/** Extra feedback rendered below the field. Bring your own `Form.Control.Feedback`. */
extraFeedback?: ReactNode;
/** `true` to adjust the vertical alignment of the left column label for checkboxes/radios. */
checkAlign?: boolean;
/** Passed as `label` to the field component. Intended for checkboxes. */
checkLabel?: ReactNode;
/** The component or element to use as the field. Passed to Formik's `Field`. */
Expand All @@ -26,6 +16,7 @@ type Props = {
children?: ReactNode;
};

/** FieldRow for use with formik */
export default function FieldRow<P = unknown>({
name,
label = '',
Expand All @@ -39,7 +30,7 @@ export default function FieldRow<P = unknown>({
children,
...props
}: Props & P) {
const { errors } = useFormikContext<any>();
const [, { error }] = useField<any>(name);

let field: ReactNode;
if (children) {
Expand All @@ -48,24 +39,21 @@ export default function FieldRow<P = unknown>({
// Checkboxes have two labels: in the left column and next to the checkbox. Form.Check handles the latter for us
// and calls it "label", but we still want to call the other one "label" for all other types of field. Therefore
// we pass "checkLabel" to the field here.
let fieldProps = props;
if (checkLabel !== undefined) {
fieldProps = { ...props, label: checkLabel };
}
field = <Field as={as} name={name} required={required} {...fieldProps} />;
const overrideProps = checkLabel !== undefined ? { label: checkLabel } : {};
field = <Field as={as} name={name} required={required} {...props} {...overrideProps} />;
}

return (
<Form.Group as={Row} controlId={name}>
<Form.Label column sm="3" data-required={required} className={checkAlign ? 'pt-0' : ''}>{label}</Form.Label>
<Col sm="9">
{field}
<Form.Control.Feedback type="invalid">
{errors[name] && (alternateError || errors[name])}
</Form.Control.Feedback>
{extraFeedback}
{help && <Form.Text muted>{help}</Form.Text>}
</Col>
</Form.Group>
<BaseFieldRow
name={name}
label={label}
help={help}
required={required}
error={error && (alternateError || error)}
extraFeedback={extraFeedback}
checkAlign={checkAlign}
>
{field}
</BaseFieldRow>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import React, { ComponentType, ReactNode } from 'react';

import { Form } from 'react-bootstrap';
import { useField, UseFieldConfig } from 'react-final-form';

import BaseFieldRow, { BaseFieldRowProps } from '../BaseFieldRow';

type Props = Omit<BaseFieldRowProps, 'error' | 'children'> & Pick<UseFieldConfig<any>, 'type'> & {
/** Overrides the real error message if the field has errors. */
alternateError?: string;
/** Passed as `label` to the field component. Intended for checkboxes. */
checkLabel?: ReactNode;
/** The component or element to use as the field. Passed to Formik's `Field`. */
as?: ComponentType<any> | string;
/** useField() config. */
config?: UseFieldConfig<any>;
/** If given, this is used as the field. */
children?: ReactNode;
};

/** FieldRow for use with react-final-form */
export default function FinalFieldRow<P = unknown>({
name,
label = '',
help,
required = false,
alternateError,
extraFeedback,
checkAlign,
checkLabel,
as: Component = Form.Control,
children,
type,
config,
...props
}: Props & P) {
const { input, meta: { error, invalid } } = useField(name, { type, ...config });

let field: ReactNode;
if (children) {
field = children;
} else {
// Checkboxes have two labels: in the left column and next to the checkbox. Form.Check handles the latter for us
// and calls it "label", but we still want to call the other one "label" for all other types of field. Therefore
// we pass "checkLabel" to the field here.
const overrideProps = checkLabel !== undefined ? { label: checkLabel } : {};
field = <Component required={required} {...props} {...input} {...overrideProps} />;
}

return (
<BaseFieldRow
name={name}
label={label}
help={help}
required={required}
error={invalid && (alternateError || error)}
extraFeedback={extraFeedback}
checkAlign={checkAlign}
>
{field}
</BaseFieldRow>
);
}
1 change: 1 addition & 0 deletions packages/ilmomasiina-components/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,4 @@ export { default as SingleEvent } from './routes/SingleEvent';
export { default as EditSignup } from './routes/EditSignup';

export { default as FieldRow } from './components/FieldRow';
export { default as FinalFieldRow } from './components/FinalFieldRow';
10 changes: 6 additions & 4 deletions packages/ilmomasiina-frontend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,18 +23,19 @@
"@types/node": "^16.11.41",
"@types/node-sass": "^4.11.2",
"@types/react": "^17.0.47",
"@types/react-csv": "^1.1.2",
"@types/react-datepicker": "^4.4.2",
"@types/react-dom": "^17.0.17",
"@types/react-redux": "^7.1.24",
"@types/react-router-dom": "^5.3.3",
"@welldone-software/why-did-you-render": "^6.2.3",
"bootstrap": "^4.6.1",
"connected-react-router": "^6.9.2",
"csv-stringify": "^6.4.2",
"date-fns": "^2.28.0",
"esbuild": "^0.14.46",
"esbuild-sass-plugin": "^2.2.6",
"formik": "^2.2.9",
"final-form": "^4.20.10",
"final-form-arrays": "^3.1.0",
"formik": "^2.4.5",
"history": "^4.10.1",
"i18next": "^22.4.11",
"i18next-browser-languagedetector": "^7.0.1",
Expand All @@ -43,9 +44,10 @@
"moment-timezone": "^0.5.34",
"react": "^17.0.2",
"react-bootstrap": "^1.6.5",
"react-csv": "^2.2.2",
"react-datepicker": "^4.8.0",
"react-dom": "^17.0.2",
"react-final-form": "^6.5.9",
"react-final-form-arrays": "^3.1.4",
"react-i18next": "^12.2.0",
"react-redux": "^7.2.8",
"react-router": "^5.3.3",
Expand Down
6 changes: 0 additions & 6 deletions packages/ilmomasiina-frontend/src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,6 @@ import { configure } from '@tietokilta/ilmomasiina-components';
import AppContainer from './containers/AppContainer';
import { apiUrl } from './paths';

if (!PROD) {
// eslint-disable-next-line global-require
const whyDidYouRender = require('@welldone-software/why-did-you-render');
whyDidYouRender(React);
}

if (PROD && SENTRY_DSN) {
Sentry.init({ dsn: SENTRY_DSN });
}
Expand Down
2 changes: 2 additions & 0 deletions packages/ilmomasiina-frontend/src/locales/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@
"editor.emails.verificationEmail": "Confirmation email",
"editor.signups.noSignups": "There are currently no signups for this event. When people sign up, they will appear here.",
"editor.signups.download": "Download list of participants",
"editor.signups.download.filename": "{{event}} participants.csv",
"editor.signups.column.firstName": "First name",
"editor.signups.column.lastName": "Last name",
"editor.signups.column.email": "Email",
Expand All @@ -189,6 +190,7 @@
"editor.signups.column.delete": "Delete",
"editor.signups.action.delete": "Delete",
"editor.signups.action.delete.confirm": "Are you sure? This cannot be undone.",
"editor.signups.unconfirmed": "Unconfirmed",
"editor.editConflict.title": "Conflicting edits",
"editor.editConflict.info1": "Another user or tab has edited this event at <1>{{time}}</1>.",
"editor.editConflict.info1.withDeleted": "Another user or tab has edited this event at <1>{{time}}</1> and deleted the following quotas or questions:",
Expand Down
2 changes: 2 additions & 0 deletions packages/ilmomasiina-frontend/src/locales/fi.json
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@
"editor.emails.verificationEmail": "Vahvistusviesti sähköpostiin",
"editor.signups.noSignups": "Tapahtumaan ei vielä ole yhtään ilmoittautumista. Kun tapahtumaan tulee ilmoittautumisia, näet ne tästä.",
"editor.signups.download": "Lataa osallistujalista",
"editor.signups.download.filename": "{{event}} osallistujalista.csv",
"editor.signups.column.firstName": "Etunimi",
"editor.signups.column.lastName": "Sukunimi",
"editor.signups.column.email": "Sähköposti",
Expand All @@ -191,6 +192,7 @@
"editor.signups.column.delete": "Poista",
"editor.signups.action.delete": "Poista",
"editor.signups.action.delete.confirm": "Oletko varma? Poistamista ei voi perua.",
"editor.signups.unconfirmed": "Vahvistamatta",
"editor.editConflict.title": "Päällekkäinen muokkaus",
"editor.editConflict.info1": "Toinen käyttäjä tai välilehti on muokannut tätä tapahtumaa <1>{{time}}</1>.",
"editor.editConflict.info1.withDeleted": "Toinen käyttäjä tai välilehti on muokannut tätä tapahtumaa <1>{{time}}</1> ja poistanut seuraavat kiintiöt tai kysymykset:",
Expand Down
8 changes: 2 additions & 6 deletions packages/ilmomasiina-frontend/src/modules/editor/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -256,21 +256,17 @@ export const publishNewEvent = (data: EditorEvent) => async (dispatch: DispatchA
export const publishEventUpdate = (
id: EventID,
data: EditorEvent,
moveSignupsToQueue: boolean = false,
) => async (dispatch: DispatchAction, getState: GetState) => {
dispatch(saving());

const cleaned = editorEventToServer(data);
const body = editorEventToServer(data);
const { accessToken } = getState().auth;

try {
const response = await adminApiFetch(`admin/events/${id}`, {
accessToken,
method: 'PATCH',
body: {
...cleaned,
moveSignupsToQueue,
},
body,
}, dispatch) as AdminEventResponse;
dispatch(loaded(response));
return response;
Expand Down

This file was deleted.

Loading

0 comments on commit 60d8a42

Please sign in to comment.