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

feat: change email updating workflow [DHIS2-18493] #1470

Merged
merged 6 commits into from
Jan 23, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
feat: clean up
  • Loading branch information
tomzemp committed Jan 13, 2025
commit 38b79c991053e3955eb5ebef530abe0d5e9c1d02
37 changes: 23 additions & 14 deletions i18n/en.pot
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ msgstr ""
"Content-Type: text/plain; charset=utf-8\n"
"Content-Transfer-Encoding: 8bit\n"
"Plural-Forms: nplurals=2; plural=(n != 1)\n"
"POT-Creation-Date: 2025-01-09T15:27:50.496Z\n"
"PO-Revision-Date: 2025-01-09T15:27:50.496Z\n"
"POT-Creation-Date: 2025-01-13T13:12:31.582Z\n"
"PO-Revision-Date: 2025-01-13T13:12:31.583Z\n"

msgid "Never"
msgstr "Never"
Expand Down Expand Up @@ -355,30 +355,42 @@ msgstr "Email is invalid"
msgid "Emails must match"
msgstr "Emails must match"

msgid "Remove email"
msgstr "Remove email"

msgid "Your email is currently verified"
msgstr "Your email is currently verified"

msgid "Are you sure you want to remove your email?"
msgstr "Are you sure you want to remove your email?"

msgid "Cancel"
msgstr "Cancel"

msgid "Email"
msgstr "Email"

msgid "Update email"
msgstr "Update email"
msgid "Change email"
msgstr "Change email"

msgid "Your email is currently verified"
msgstr "Your email is currently verified"
msgid "There is no email to remove"
msgstr "There is no email to remove"

msgid "If you change your email, you may need to reverify your email."
msgstr "If you change your email, you may need to reverify your email."

msgid "Current email"
msgstr "Current email"

msgid "no current email"
msgstr "no current email"

msgid "Enter new email"
msgstr "Enter new email"

msgid "Confirm new email"
msgstr "Confirm new email"

msgid "Cancel"
msgstr "Cancel"

msgid "Save"
msgstr "Save"

Expand Down Expand Up @@ -409,8 +421,8 @@ msgstr "Email verification link sent successfully!"
msgid "Failed to send email verification link."
msgstr "Failed to send email verification link."

msgid "Verify Email"
msgstr "Verify Email"
msgid "Verify email"
msgstr "Verify email"

msgid ""
"Your email is not verified. Please verify your email to continue using the "
Expand Down Expand Up @@ -640,9 +652,6 @@ msgstr "Other"
msgid "E-mail"
msgstr "E-mail"

msgid "E-mail Verification"
msgstr "E-mail Verification"

msgid "Mobile phone number"
msgstr "Mobile phone number"

Expand Down
10 changes: 0 additions & 10 deletions src/layout/FormFields.component.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import userSettingsKeyMapping from '../userSettingsMapping.js'
import AvatarEditor from './AvatarEditor.component.js'
import { ModalField } from './ModalField.component.js'
import AppTheme from './theme.js'
import { VerifyEmail } from './VerifyEmail.component.js'
import { VerifyEmailWarning } from './VerifyEmailWarning.js'

const styles = {
Expand Down Expand Up @@ -237,13 +236,6 @@ function createAvatarEditor(fieldBase, d2, valueStore) {
})
}

function createVerifyButton(fieldBase, valueStore) {
return Object.assign({}, fieldBase, {
component: VerifyEmail,
props: { userEmail: valueStore.state['email'] || '' },
})
}

function createModalField({ fieldBase, valueStore, onUpdate, d2 }) {
return Object.assign({}, fieldBase, {
component: ModalField,
Expand Down Expand Up @@ -310,8 +302,6 @@ function createField({ fieldName, valueStore, d2, onUpdate }) {
return createAccountEditor(fieldBase, d2, valueStore)
case 'avatar':
return createAvatarEditor(fieldBase, d2, valueStore)
case 'submit':
return createVerifyButton(fieldBase, valueStore)
case 'modal':
return createModalField({ fieldBase, valueStore, onUpdate, d2 })
default:
Expand Down
95 changes: 86 additions & 9 deletions src/layout/ModalField.component.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import TextField from 'd2-ui/lib/form-fields/TextField'
import PropTypes from 'prop-types'
import React, { useMemo, useState } from 'react'
import styles from './ModalField.component.module.css'
import { VerifyEmail } from './VerifyEmail.component.js'

const TooltipWrapper = ({ disabled, content, children }) => {
if (!disabled) {
Expand All @@ -39,16 +40,67 @@ const getSaveDisabledContent = ({ newEmail, emailValidationMessage }) => {
return i18n.t('Emails must match')
}

const RemoveModal = ({
removeModalOpen,
closeModal,
userEmailVerified,
setUserEmail,
onUpdate,
}) => (
<Modal hide={!removeModalOpen} onClose={closeModal}>
<ModalTitle>{i18n.t('Remove email')}</ModalTitle>

<ModalContent>
{userEmailVerified && (
<NoticeBox
className={styles.emailModalItem}
title={i18n.t('Your email is currently verified')}
warning
></NoticeBox>
)}
<div>{i18n.t('Are you sure you want to remove your email?')}</div>
</ModalContent>

<ModalActions>
<ButtonStrip end>
<Button onClick={() => closeModal()} secondary>
{i18n.t('Cancel')}
</Button>

<Button
onClick={() => {
setUserEmail(null)
onUpdate('email', '')
closeModal()
}}
destructive
>
{i18n.t('Remove email')}
</Button>
</ButtonStrip>
</ModalActions>
</Modal>
)

RemoveModal.propTypes = {
closeModal: PropTypes.func,
removeModalOpen: PropTypes.bool,
setUserEmail: PropTypes.bool,
userEmailVerified: PropTypes.bool,
onUpdate: PropTypes.func,
}

export function ModalField({
Copy link
Member

@Birkbjo Birkbjo Jan 13, 2025

Choose a reason for hiding this comment

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

I'm a bit confused about the naming of this component (and file). When I read "ModalField" I would expect this to be a generic field that somehow opens a modal. I would expect this to be called EmailField.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've updated to use EmailField, and to make some of the related functions/props correspond to this update

userEmail,
userEmailVerified,
setUserEmail,
onUpdate,
}) {
const [modalOpen, setModalOpen] = useState()
const [removeModalOpen, setRemoveModalOpen] = useState()
const [newEmail, setNewEmail] = useState()
const [newEmailConfirm, setNewEmailConfirm] = useState()
const [newEmailTouched, setNewEmailTouched] = useState(false)
const [newEmailConfirmTouched, setNewEmailConfirmTouched] = useState(false)
const emailValidationMessage = useMemo(
() => emailValidator(newEmail),
[newEmail]
Expand All @@ -63,9 +115,10 @@ export function ModalField({

const closeModal = () => {
setModalOpen(false)
setRemoveModalOpen(false)
setNewEmail()
setNewEmailConfirm()
setNewEmailTouched(false)
setNewEmailConfirmTouched(false)
}
return (
<div className={styles.emailModalContainer}>
Expand All @@ -75,13 +128,26 @@ export function ModalField({
floatingLabelText={i18n.t('Email')}
style={{ width: '100%' }}
/>
<div>
<div className={styles.buttonContainer}>
<VerifyEmail userEmail={userEmail} />
<Button secondary onClick={() => setModalOpen(true)}>
{i18n.t('Update email')}
{i18n.t('Change email')}
</Button>
<TooltipWrapper
disabled={!userEmail}
content={i18n.t('There is no email to remove')}
>
<Button
destructive
onClick={() => setRemoveModalOpen(true)}
disabled={!userEmail}
>
{i18n.t('Remove email')}
</Button>
</TooltipWrapper>
</div>
<Modal hide={!modalOpen} onClose={closeModal}>
Copy link
Member

@Birkbjo Birkbjo Jan 13, 2025

Choose a reason for hiding this comment

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

Kind of minor. But I think I would've preferred this (The modal with the contents) to be it's own component. Not just for the sake of extracting this out - but it would also mean that we don't have to manually reset the state of this modal during closeModal, if we were to conditionally render it.
I haven't done a full analysis of what it would look like, or if would be annoying to do - just an observation.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that's fair. We just started writing it this way and then it was easier to continue on with what we had and do a manual reset of the component state. But it became a bit longer than what we had originally, so seems fair to also break it up into a separate component. I've updated to put it in its own component but within the EmailField.js file as I think it's functionally easier to read the code when this is in one file.

<ModalTitle>{i18n.t('Update email')}</ModalTitle>
<ModalTitle>{i18n.t('Change email')}</ModalTitle>

<ModalContent>
{userEmailVerified && (
Expand All @@ -98,7 +164,11 @@ export function ModalField({

<InputField
label={i18n.t('Current email')}
value={userEmail}
value={
userEmail !== ''
? userEmail
: i18n.t('no current email')
}
type="email"
disabled
className={styles.emailModalItem}
Expand All @@ -117,14 +187,14 @@ export function ModalField({
label={i18n.t('Confirm new email')}
value={newEmailConfirm}
type="email"
error={newEmailTouched && !emailsMatch}
error={newEmailConfirmTouched && !emailsMatch}
validationText={
emailsMatch || !newEmailTouched
emailsMatch || !newEmailConfirmTouched
? undefined
: i18n.t('Emails must match')
}
onChange={(newValue) => {
setNewEmailTouched(true)
setNewEmailConfirmTouched(true)
setNewEmailConfirm(newValue.value)
}}
className={styles.emailModalItem}
Expand Down Expand Up @@ -156,6 +226,13 @@ export function ModalField({
</ButtonStrip>
</ModalActions>
</Modal>
<RemoveModal
removeModalOpen={removeModalOpen}
closeModal={closeModal}
userEmailVerified={userEmailVerified}
setUserEmail={setUserEmail}
onUpdate={onUpdate}
/>
</div>
)
}
Expand Down
5 changes: 5 additions & 0 deletions src/layout/ModalField.component.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,9 @@

.emailModalItem {
margin-block-end: 16px;
}

.buttonContainer {
display: flex;
gap: 8px;
}
18 changes: 8 additions & 10 deletions src/layout/VerifyEmail.component.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,16 +41,14 @@ export function VerifyEmail({ userEmail }) {
}

return (
<div style={{ display: 'flex', alignItems: 'center', gap: '10px' }}>
<Button
secondary
onClick={mutateEmailVerification}
disabled={mutationLoading || isInvalidEmail || !userEmail}
loading={mutationLoading}
>
{i18n.t('Verify email')}
</Button>
</div>
<Button
secondary
onClick={mutateEmailVerification}
disabled={mutationLoading || isInvalidEmail || !userEmail}
loading={mutationLoading}
>
{i18n.t('Verify email')}
</Button>
)
}

Expand Down
1 change: 0 additions & 1 deletion src/profile/Profile.component.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ function EditProfile() {
'firstName',
'surname',
'email',
'emailVerification',
'avatar',
'phoneNumber',
'introduction',
Expand Down
5 changes: 0 additions & 5 deletions src/userSettingsMapping.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,6 @@ const settingsKeyMapping = {
label: i18n.t('E-mail'),
type: 'modal',
},
emailVerification: {
name: 'emailVerification',
label: i18n.t('E-mail Verification'),
type: 'submit',
},
phoneNumber: {
label: i18n.t('Mobile phone number'),
type: 'textfield',
Expand Down
Loading