From c6b8c77baafc9b5fbe91cad9940cea6b0d2ecdb2 Mon Sep 17 00:00:00 2001 From: Martin Krulis Date: Sun, 12 May 2019 17:04:51 +0200 Subject: [PATCH] Fixing bugs, minor ux issues, refactoring, cleanups, and removing ugly hacks. --- .../MessagesList/MessagesList.js | 52 +++++-------- .../MessagesList/MessagesList.less | 12 +-- .../EditSystemMessageForm.js | 51 ++++++------- .../FilterSystemMessagesForm.js | 76 ++++++++----------- src/components/helpers/usersRoles.js | 9 +++ .../HeaderNotificationsDropdown.js | 9 ++- .../HeaderSystemMessagesDropdown.js | 10 ++- .../HeaderSystemMessagesDropdown.less | 6 +- .../HeaderNotificationsContainer.js | 29 +------ .../HeaderSystemMessagesContainer.js | 30 +------- src/locales/cs.json | 2 +- src/pages/SystemMessages/SystemMessages.js | 24 +++--- src/redux/reducer.js | 6 +- 13 files changed, 129 insertions(+), 187 deletions(-) diff --git a/src/components/SystemMessages/MessagesList/MessagesList.js b/src/components/SystemMessages/MessagesList/MessagesList.js index 5c459324e..b538a58d3 100644 --- a/src/components/SystemMessages/MessagesList/MessagesList.js +++ b/src/components/SystemMessages/MessagesList/MessagesList.js @@ -7,24 +7,25 @@ import { defaultMemoize } from 'reselect'; import SortableTable, { SortableTableColumnDescriptor } from '../../widgets/SortableTable'; import { getLocalizedText } from '../../../helpers/localizedData'; import DateTime from '../../widgets/DateTime'; -import { roleLabels } from '../../helpers/usersRoles'; +import { roleLabels, rolePriorities } from '../../helpers/usersRoles'; import UsersNameContainer from '../../../containers/UsersNameContainer'; import FilterSystemMessagesForm from '../../forms/FilterSystemMessagesForm/FilterSystemMessagesForm'; import styles from './MessagesList.less'; -const getVisibleMessages = (systemMessages, showAll) => - showAll ? systemMessages : systemMessages.filter(msg => msg.visibleTo * 1000 >= Date.now()); +const prepareData = defaultMemoize((systemMessages, showAll, renderActions) => { + const filteredMessages = showAll ? systemMessages : systemMessages.filter(msg => msg.visibleTo * 1000 >= Date.now()); + + return filteredMessages.map(message => ({ + ...message, + text: { localizedTexts: message.localizedTexts }, + buttons: renderActions && renderActions(message), + })); +}); class MessagesList extends Component { state = { showAll: false, visibleMessages: [] }; - componentDidMount() { - this.setState({ - visibleMessages: getVisibleMessages(this.props.systemMessages, this.state.showAll), - }); - } - prepareColumnDescriptors = defaultMemoize(locale => { const columns = [ new SortableTableColumnDescriptor( @@ -44,6 +45,7 @@ class MessagesList extends Component { { comparator: ({ visibleFrom: f1 }, { visibleFrom: f2 }) => f2 - f1, cellRenderer: visibleFrom => visibleFrom && , + className: 'valign-middle', } ), @@ -53,19 +55,22 @@ class MessagesList extends Component { { comparator: ({ visibleTo: t1 }, { visibleTo: t2 }) => t2 - t1, cellRenderer: visibleTo => visibleTo && , + className: 'valign-middle', } ), new SortableTableColumnDescriptor('authorId', , { - cellRenderer: authorId => authorId && , + cellRenderer: authorId => authorId && , + className: 'valign-middle', }), new SortableTableColumnDescriptor( 'role', , { - comparator: ({ role: r1 }, { role: r2 }) => r1.localeCompare(r2, locale), + comparator: ({ role: r1 }, { role: r2 }) => Number(rolePriorities[r2]) - Number(rolePriorities[r1]), cellRenderer: role => role && roleLabels[role], + className: 'valign-middle', } ), @@ -75,37 +80,22 @@ class MessagesList extends Component { { comparator: ({ type: t1 }, { type: t2 }) => t1.localeCompare(t2, locale), cellRenderer: type => type && , + className: 'text-center valign-middle', } ), new SortableTableColumnDescriptor('buttons', '', { - className: 'text-right', + className: 'text-right valign-middle', }), ]; return columns; }); - prepareData = defaultMemoize(systemMessages => { - const { renderActions } = this.props; - - return systemMessages.map(message => { - const data = { - text: { localizedTexts: message.localizedTexts }, - visibleFrom: message.visibleFrom, - visibleTo: message.visibleTo, - authorId: message.authorId, - role: message.role, - type: message.type, - buttons: renderActions && renderActions(message), - }; - return data; - }); - }); - render() { const { systemMessages, + renderActions, intl: { locale }, } = this.props; @@ -115,17 +105,17 @@ class MessagesList extends Component { onSubmit={({ showAll }) => { this.setState({ showAll: showAll, - visibleMessages: getVisibleMessages(systemMessages, showAll), }); return Promise.resolve(); }} initialValues={{ showAll: this.state.showAll }} /> +
+ Object.keys(roleLabelsSimpleMessages).map(role => ({ + key: role, + name: formatMessage(roleLabelsSimpleMessages[role]), + })) +); + const EditSystemMessageForm = ({ - initialValues, error, dirty, submitting, @@ -30,15 +36,19 @@ const EditSystemMessageForm = ({ submitFailed, submitSucceeded, invalid, - asyncValidating, isOpen, onClose, - intl: { formatMessage }, + createNew = false, + intl: { locale, formatMessage }, }) => ( - + {createNew ? ( + + ) : ( + + )} @@ -54,18 +64,13 @@ const EditSystemMessageForm = ({ name="type" component={SelectField} options={typeOptions} - addEmptyOption label={} /> ({ - key: role, - name: formatMessage(roleLabelsSimpleMessages[role]), - }))} - addEmptyOption + options={getRoleOptions(formatMessage)} label={ , submitting: , @@ -128,18 +132,16 @@ const EditSystemMessageForm = ({ EditSystemMessageForm.propTypes = { error: PropTypes.any, - initialValues: PropTypes.object.isRequired, - values: PropTypes.object, handleSubmit: PropTypes.func.isRequired, dirty: PropTypes.bool, submitting: PropTypes.bool, submitFailed: PropTypes.bool, submitSucceeded: PropTypes.bool, invalid: PropTypes.bool, - asyncValidating: PropTypes.oneOfType([PropTypes.bool, PropTypes.string]), links: PropTypes.object, isOpen: PropTypes.bool.isRequired, onClose: PropTypes.func.isRequired, + createNew: PropTypes.bool, intl: intlShape.isRequired, }; @@ -160,7 +162,7 @@ const validate = ({ localizedTexts, type, role, visibleFrom, visibleTo }) => { }); if (!visibleFrom) { - errors['visibleFrom'] = ( + errors.visibleFrom = ( { } if (!visibleTo) { - errors['visibleTo'] = ( + errors.visibleTo = ( { ); } - if (visibleTo.isBefore(moment.unix(Date.now() / 1000))) { - errors['visibleTo'] = ( - - ); - } - if (visibleTo.isBefore(visibleFrom)) { - errors['visibleFrom'] = ( + errors.visibleFrom = ( { } if (!type) { - errors['type'] = ( + errors.type = ( { } if (!role) { - errors['role'] = ( + errors.role = ( (
- - {submitFailed && ( - - - - )} - - - - - - - } + + + + - - + } + /> + - -
- , - success: , - }} - /> -
- - - - + +
+ , + success: , + }} + /> +
+ + +
); diff --git a/src/components/helpers/usersRoles.js b/src/components/helpers/usersRoles.js index 462b29107..e0c985242 100644 --- a/src/components/helpers/usersRoles.js +++ b/src/components/helpers/usersRoles.js @@ -78,6 +78,15 @@ export const roleDescriptions = { export const knownRoles = Object.keys(roleLabels); +// Ordering from lowest to highest power (when roles ordering is required) +export const rolePriorities = { + [STUDENT_ROLE]: 1, + [SUPERVISOR_STUDENT_ROLE]: 2, + [SUPERVISOR_ROLE]: 3, + [EMPOWERED_SUPERVISOR_ROLE]: 4, + [SUPERADMIN_ROLE]: Number.MAX_SAFE_INTEGER, +}; + /* * Helper Functions */ diff --git a/src/components/widgets/HeaderNotificationsDropdown/HeaderNotificationsDropdown.js b/src/components/widgets/HeaderNotificationsDropdown/HeaderNotificationsDropdown.js index 91b15c629..766c7ac0b 100644 --- a/src/components/widgets/HeaderNotificationsDropdown/HeaderNotificationsDropdown.js +++ b/src/components/widgets/HeaderNotificationsDropdown/HeaderNotificationsDropdown.js @@ -10,12 +10,13 @@ import HeaderNotification from '../HeaderNotification'; import styles from './headerNotificationDropdown.less'; +const preventClickPropagation = ev => ev.stopPropagation(); + const HeaderNotificationsDropdown = ({ isOpen, showAll, toggleOpen, toggleShowAll, - markClick, hideNotification, newNotifications, oldNotifications, @@ -25,7 +26,8 @@ const HeaderNotificationsDropdown = ({ 'notifications-menu': true, dropdown: true, open: isOpen, - })}> + })} + onMouseDown={isOpen ? preventClickPropagation : undefined}> {newNotifications.size === 0 ? ( @@ -36,7 +38,7 @@ const HeaderNotificationsDropdown = ({ )} -
    +
    • ( +const preventClickPropagation = ev => ev.stopPropagation(); + +const HeaderSystemMessagesDropdown = ({ isOpen, toggleOpen, systemMessages, intl: { locale } }) => (
    • + })} + onMouseDown={isOpen ? preventClickPropagation : undefined}> {systemMessages.length > 0 && } -
        +
        • { - this.lastClick = 0; if (canUseDOM) { - window.addEventListener('click', () => this.clickAnywhere()); + window.addEventListener('mousedown', this.close); } }; componentWillUnMount = () => { if (canUseDOM) { - window.removeEventListener(() => this.clickAnywhere()); - } - }; - - clickAnywhere = () => { - if (this.state.isOpen === true && this.isClickingSomewhereElse()) { - this.close(); + window.removeEventListener('mousedown', this.close); } }; - markClick = () => { - this.lastClick = Date.now(); - }; - - /** - * Determines, whether this click is on the container or not - a 10ms tolerance - * between now and the time of last click on the container is defined. - */ - isClickingSomewhereElse = () => Date.now() - this.lastClick > 50; - - // - // - toggleOpen = e => { e.preventDefault(); this.state.isOpen ? this.close() : this.open(); - this.markClick(); }; toggleShowAll = e => { e.preventDefault(); - this.markClick(); this.setState({ showAll: !this.state.showAll }); }; @@ -79,7 +55,6 @@ class HeaderNotificationsContainer extends Component { Promise.all([dispatch(fetchAllUserMessages)]); - // // Monitor clicking and hide the notifications panel when the user clicks sideways componentWillMount = () => { this.props.loadAsync(); - this.lastClick = 0; if (canUseDOM) { - window.addEventListener('click', () => this.clickAnywhere()); + window.addEventListener('mousedown', this.close); } }; componentWillUnMount = () => { if (canUseDOM) { - window.removeEventListener(() => this.clickAnywhere()); + window.removeEventListener('mousedown', this.close); } }; - clickAnywhere = () => { - if (this.state.isOpen === true && this.isClickingSomewhereElse()) { - this.close(); - } - }; - - markClick = () => { - this.lastClick = Date.now(); - }; - - /** - * Determines, whether this click is on the container or not - a 50ms tolerance - * between now and the time of last click on the container is defined. - */ - isClickingSomewhereElse = () => Date.now() - this.lastClick > 50; - toggleOpen = e => { e.preventDefault(); this.state.isOpen ? this.close() : this.open(); - this.markClick(); }; close = () => { @@ -64,12 +45,7 @@ class HeaderSystemMessagesContainer extends Component { return ( }> {() => ( - + )} ); diff --git a/src/locales/cs.json b/src/locales/cs.json index 85143c5be..ce1302cb1 100644 --- a/src/locales/cs.json +++ b/src/locales/cs.json @@ -1235,7 +1235,7 @@ "app.systemMessages.title": "Systémové zprávy", "app.systemMessagesList.noMessages": "Nyní zde nejsou žádné systémové zprávy.", "app.systemMessagesList.role": "Role", - "app.systemMessagesList.showAll": "Ukaž všechny zprávy (včetně exporovaných)", + "app.systemMessagesList.showAll": "Zobrazit všechny zprávy (včetně expirovaných)", "app.systemMessagesList.text": "Text", "app.systemMessagesList.type": "Typ", "app.systemMessagesList.visibleFrom": "Viditelné od", diff --git a/src/pages/SystemMessages/SystemMessages.js b/src/pages/SystemMessages/SystemMessages.js index 66e68594b..672243292 100644 --- a/src/pages/SystemMessages/SystemMessages.js +++ b/src/pages/SystemMessages/SystemMessages.js @@ -20,17 +20,19 @@ const localizedTextDefaults = { text: '', }; -const messageInitialValues = () => ({ +const newMessageInitialValues = { localizedTexts: getLocalizedTextsInitialValues([], localizedTextDefaults), groupsIds: [], id: undefined, + type: 'success', + role: 'student', visibleFrom: moment(), visibleTo: moment() .add(1, 'week') .endOf('day'), -}); +}; -const messageToForm = message => { +const getMessageInitialValues = message => { const processedData = Object.assign({}, message, { visibleFrom: moment.unix(message.visibleFrom), visibleTo: moment.unix(message.visibleTo), @@ -39,7 +41,7 @@ const messageToForm = message => { return processedData; }; -const messageToSubmit = data => +const transformMessageFormData = data => Object.assign({}, data, { visibleFrom: moment(data.visibleFrom).unix(), visibleTo: moment(data.visibleTo).unix(), @@ -49,10 +51,11 @@ const messageToSubmit = data => class SystemMessages extends Component { state = { isOpen: false, - message: messageInitialValues(), + createNew: false, + message: newMessageInitialValues, }; - formReset = () => this.setState({ isOpen: false, message: messageInitialValues() }); + formReset = () => this.setState({ isOpen: false, message: newMessageInitialValues }); static loadAsync = (params, dispatch) => Promise.all([dispatch(fetchAllMessages)]); @@ -114,7 +117,7 @@ class SystemMessages extends Component { unlimitedHeight footer={

          - @@ -128,7 +131,7 @@ class SystemMessages extends Component { bsSize="xs" bsStyle="warning" onClick={() => { - this.setState({ isOpen: true, message: messageToForm(message) }); + this.setState({ isOpen: true, createNew: false, message: getMessageInitialValues(message) }); }}> @@ -139,6 +142,7 @@ class SystemMessages extends Component { /> this.formReset()} @@ -171,7 +175,7 @@ export default connect( }), dispatch => ({ loadAsync: () => SystemMessages.loadAsync({}, dispatch), - createMessage: data => dispatch(createMessage(messageToSubmit(data))), - editMessage: (id, data) => dispatch(editMessage(id, messageToSubmit(data))), + createMessage: data => dispatch(createMessage(transformMessageFormData(data))), + editMessage: (id, data) => dispatch(editMessage(id, transformMessageFormData(data))), }) )(SystemMessages); diff --git a/src/redux/reducer.js b/src/redux/reducer.js index 1f4e38372..b5c5558b4 100644 --- a/src/redux/reducer.js +++ b/src/redux/reducer.js @@ -60,6 +60,7 @@ const createRecodexReducers = (token, instanceId) => ({ attachmentFiles, auth: auth(token, instanceId), boxes, + broker, canSubmit: canSubmitModule, comments, emailVerification, @@ -101,11 +102,10 @@ const createRecodexReducers = (token, instanceId) => ({ submissionEvaluations, submissionFailures, supplementaryFiles, + systemMessages, + upload, users, userSwitching, - upload, - broker, - systemMessages, }); const librariesReducers = {