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

Remove save button in user settings #1030

Merged
merged 25 commits into from
Oct 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
e412ed8
refactor(stacked-panes): Separate layout code from save/cancel buttons.
binh-dam-ibigroup Oct 9, 2023
c93a6ca
refactor(stacked-panes): Add more types to component.
binh-dam-ibigroup Oct 11, 2023
2874d67
refactor(existing-account-display): Submit changes right away.
binh-dam-ibigroup Oct 11, 2023
ae94f94
refactor(existing-account-display): Fix types
binh-dam-ibigroup Oct 11, 2023
1af40ce
refactor(notification-prefs-pane): Set onChange handler explicitly.
binh-dam-ibigroup Oct 11, 2023
263c940
refactor(notification-prefs-pane): Add missing i18n
binh-dam-ibigroup Oct 11, 2023
9a5ffc7
refactor(user-account-screen): Remove form element when viewing exist…
binh-dam-ibigroup Oct 11, 2023
06b9bc6
refactor(user-account-screen): Remove unused validation.
binh-dam-ibigroup Oct 11, 2023
9783e83
improvement(existing-account-display): Display toast to confirm field…
binh-dam-ibigroup Oct 12, 2023
9904996
style(existing-account-display): Clean up code.
binh-dam-ibigroup Oct 12, 2023
57a9b68
refactor(user-account-screen): Convert to TypeScript
binh-dam-ibigroup Oct 12, 2023
f482067
Merge branch 'dev' into remove-user-setting-save-btn
binh-dam-ibigroup Oct 12, 2023
15cd0be
chore(i18n): Add i18n group exceptions.
binh-dam-ibigroup Oct 12, 2023
760539b
improvement(existing-account-display): Disable inputs during user dat…
binh-dam-ibigroup Oct 16, 2023
fafbe96
improvement(user-account-screen): Consolidate input change handling.
binh-dam-ibigroup Oct 16, 2023
2052b17
refactor(user-account-screen): Fix types.
binh-dam-ibigroup Oct 16, 2023
64f98db
improvement(user-account-screen): Add i18n to profile upd error, reen…
binh-dam-ibigroup Oct 19, 2023
c9407f7
Merge branch 'dev' into remove-user-setting-save-btn
binh-dam-ibigroup Oct 19, 2023
86bd255
Merge branch 'dev' into remove-user-setting-save-btn
binh-dam-ibigroup Oct 24, 2023
9487b54
improvement(user-account-screen): Add visuals to denote submission pr…
binh-dam-ibigroup Oct 24, 2023
70b8881
improvement(user-account-screen): Apply animation styles to all brows…
binh-dam-ibigroup Oct 24, 2023
0c33524
improvement(user-account-screen): Add "updating" toast while user upd…
binh-dam-ibigroup Oct 24, 2023
eb85c70
refactor(user-account-screen): Tweak types
binh-dam-ibigroup Oct 24, 2023
b188043
improvement(user-account-screen): Add wait cursor to label of changed…
binh-dam-ibigroup Oct 25, 2023
18f430b
Merge branch 'dev' into remove-user-setting-save-btn
binh-dam-ibigroup Oct 26, 2023
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
6 changes: 6 additions & 0 deletions i18n/en-US.yml
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,7 @@ components:
description: The content you requested is not available.
header: Content not found
NotificationPrefsPane:
devicesRegistered: "{count, plural, one {# device} other {# devices}} registered"
noDeviceForPush: Register your device using the mobile app to access push notifications.
notificationChannelPrompt: "Receive notifications about your saved trips via:"
OTP2ErrorRenderer:
Expand Down Expand Up @@ -717,6 +718,11 @@ components:
confirmDelete: >-
Are you sure you would like to delete your user account? Once you do so,
it cannot be recovered.
errorUpdatingProfile: Error updating profile.
fieldUpdated: This setting has been updated.
fields:
storeTripHistory: Store trip history
updating: Updating
UserSettings:
confirmDeletion: >-
You have recent searches and/or places stored. Disabling storage of recent
Expand Down
8 changes: 8 additions & 0 deletions i18n/fr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,9 @@ components:
description: Le contenu que vous avez demandé n'est pas disponible.
header: Contenu introuvable
NotificationPrefsPane:
devicesRegistered: >-
{count, plural, one {# appareil enregistré} other {# appareils
enregistrés}}
noDeviceForPush: Inscrivez-vous avec l'application mobile pour accéder à ce paramètre.
notificationChannelPrompt: "Recevoir des notifications sur vos trajets par :"
OTP2ErrorRenderer:
Expand Down Expand Up @@ -744,6 +747,11 @@ components:
confirmDelete: >-
Voulez-vous vraiment supprimer votre compte ? Cette action est
irréversible.
errorUpdatingProfile: Erreur dans la mise à jour de votre profil.
fieldUpdated: Ce paramètre a été mis à jour.
fields:
storeTripHistory: Enregistrement des recherches
updating: Mise à jour
UserSettings:
confirmDeletion: >-
Vous avez des recherches et/ou lieux récemment enregistrés qui vont être
Expand Down
3 changes: 3 additions & 0 deletions i18n/i18n-exceptions.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
"sms",
"push"
],
"components.UserAccountScreen.fields.*": [
"storeTripHistory"
],
"components.OTP2ErrorRenderer.*.body": [
"LOCATION_NOT_FOUND",
"NO_STOPS_IN_RANGE",
Expand Down
2 changes: 1 addition & 1 deletion lib/actions/ui.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ export const resetSessionTimeout = createAction('RESET_SESSION_TIMEOUT')
* that preserves the current search or, if
* replaceSearch is provided (including an empty string), replaces the search
* when routing to a new URL path.
* @param {[type]} url path to route to
* @param {string} url path to route to
* @param {string} [replaceSearch] optional search string to replace current one
* @param {func} [routingMethod] the connected-react-router method to execute (defaults to push).
*/
Expand Down
43 changes: 23 additions & 20 deletions lib/components/user/existing-account-display.tsx
Original file line number Diff line number Diff line change
@@ -1,30 +1,38 @@
import { connect } from 'react-redux'
import { FormattedMessage, useIntl } from 'react-intl'
import { FormikProps } from 'formik'
import React from 'react'

import { AppReduxState } from '../../util/state-types'
import { TransitModeConfig } from '../../util/config-types'
import PageTitle from '../util/page-title'

import { EditedUser } from './types'
import A11yPrefs from './a11y-prefs'
import BackToTripPlanner from './back-to-trip-planner'
import DeleteUser from './delete-user'
import FavoritePlaceList from './places/favorite-place-list'
import NotificationPrefsPane from './notification-prefs-pane'
import StackedPaneDisplay from './stacked-pane-display'
import StackedPanes from './stacked-panes'
import TermsOfUsePane from './terms-of-use-pane'

interface Props extends FormikProps<EditedUser> {
wheelchairEnabled: boolean
}

/**
* This component handles the existing account display.
*/
const ExistingAccountDisplay = (props: {
onCancel: () => void
wheelchairEnabled: boolean
}) => {
const ExistingAccountDisplay = (props: Props) => {
// The props include Formik props that provide access to the current user data
// and to its own blur/change/submit event handlers that automate the state.
// We forward the props to each pane so that their individual controls
// can be wired to be managed by Formik.
const { onCancel, wheelchairEnabled } = props
const paneSequence = [

const { wheelchairEnabled } = props
const intl = useIntl()

const panes = [
{
pane: FavoritePlaceList,
props,
Expand Down Expand Up @@ -55,7 +63,6 @@ const ExistingAccountDisplay = (props: {
}
]

const intl = useIntl()
// Repeat text from the SubNav component in the title bar for brevity.
const settings = intl.formatMessage({
id: 'components.SubNav.settings'
Expand All @@ -67,25 +74,21 @@ const ExistingAccountDisplay = (props: {
<div>
<BackToTripPlanner />
<PageTitle title={[settings, myAccount]} />
<StackedPaneDisplay
onCancel={onCancel}
paneSequence={paneSequence}
<StackedPanes
panes={panes}
title={
<FormattedMessage id="components.ExistingAccountDisplay.mainTitle" />
}
/>
</div>
)
}
// TODO: state type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const mapStateToProps = (state: any) => {
const { accessModes } = state.otp.config?.modes
const wheelchairEnabled =
accessModes &&
accessModes.some(
(mode: { showWheelchairSetting: boolean }) => mode.showWheelchairSetting
)

const mapStateToProps = (state: AppReduxState) => {
const { accessModes } = state.otp.config.modes
const wheelchairEnabled = accessModes?.some(
(mode: TransitModeConfig) => mode.showWheelchairSetting
)
return {
wheelchairEnabled
}
Expand Down
6 changes: 3 additions & 3 deletions lib/components/user/monitored-trip/saved-trip-editor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import React, { ComponentType } from 'react'

import BackLink from '../back-link'
import PageTitle from '../../util/page-title'
import StackedPaneDisplay from '../stacked-pane-display'
import StackedPanesWithSave from '../stacked-panes-with-save'

interface Props {
isCreating: boolean
Expand Down Expand Up @@ -50,9 +50,9 @@ const SavedTripEditor = (props: Props): JSX.Element => {
<>
<PageTitle title={title} />
<BackLink />
<StackedPaneDisplay
<StackedPanesWithSave
onCancel={onCancel}
paneSequence={paneSequence}
panes={paneSequence}
title={title}
/>
</>
Expand Down
32 changes: 22 additions & 10 deletions lib/components/user/new-account-wizard.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { Form, FormikProps } from 'formik'
import { FormattedMessage, useIntl } from 'react-intl'
import { FormikProps } from 'formik'
import React, { useCallback } from 'react'
import toast from 'react-hot-toast'

import PageTitle from '../util/page-title'

import { User } from './types'
import { EditedUser } from './types'
import AccountSetupFinishPane from './account-setup-finish-pane'
import FavoritePlaceList from './places/favorite-place-list'
import NotificationPrefsPane from './notification-prefs-pane'
Expand All @@ -16,18 +17,20 @@ import VerifyEmailPane from './verify-email-pane'
// and to its own blur/change/submit event handlers that automate the state.
// We forward the props to each pane (via SequentialPaneDisplay) so that their individual controls
// can be wired to be managed by Formik.
type FormikUserProps = FormikProps<User>
type FormikUserProps = FormikProps<EditedUser>

interface Props extends FormikUserProps {
activePaneId: string
onCreate: (value: User) => void
onCancel: () => void
onCreate: (value: EditedUser) => void
}

/**
* This component is the new account wizard.
*/
const NewAccountWizard = ({
activePaneId,
onCancel, // provided by UserAccountScreen
onCreate, // provided by UserAccountScreen
...formikProps // provided by Formik
}: Props): JSX.Element => {
Expand All @@ -41,20 +44,28 @@ const NewAccountWizard = ({
}
}, [onCreate, userData])

const handleFinish = useCallback(() => {
// Display a toast to acknowledge saved changes
// (although in reality, changes quietly took effect in previous screens).
toast.success(intl.formatMessage({ id: 'actions.user.preferencesSaved' }))

onCancel && onCancel()
}, [intl, onCancel])

if (activePaneId === 'verify') {
const verifyEmail = intl.formatMessage({
id: 'components.NewAccountWizard.verify'
})
return (
<>
<Form id="user-settings-form" noValidate>
<PageTitle title={verifyEmail} />
<h1>{verifyEmail}</h1>
<VerifyEmailPane {...formikProps} />
</>
</Form>
)
}

const { hasConsentedToTerms, notificationChannel = 'email' } = userData
const { hasConsentedToTerms, notificationChannel = ['email'] } = userData
Copy link
Member

Choose a reason for hiding this comment

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

why was this changed to a single value array? Is this to allow for more notification channels like 'sms'

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Correct. I also changed the type of the user data to EditedUser, for which enabled notifications channels are stored in an array.

const createNewAccount = intl.formatMessage({
id: 'components.NewAccountWizard.createNewAccount'
})
Expand All @@ -72,7 +83,7 @@ const NewAccountWizard = ({
{
id: 'notifications',
invalid:
notificationChannel === 'sms' &&
notificationChannel.includes('sms') &&
(!userData.phoneNumber || !userData.isPhoneNumberVerified),
invalidMessage: intl.formatMessage({
id: 'components.PhoneNumberEditor.invalidPhone'
Expand All @@ -93,14 +104,15 @@ const NewAccountWizard = ({
]

return (
<>
<Form id="user-settings-form" noValidate>
<PageTitle title={createNewAccount} />
<SequentialPaneDisplay
activePaneId={activePaneId}
onFinish={handleFinish}
paneProps={formikProps}
panes={paneSequence}
/>
</>
</Form>
)
}

Expand Down
15 changes: 10 additions & 5 deletions lib/components/user/notification-prefs-pane.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ const NotificationOption = styled(ListGroupItem)`
*/
const NotificationPrefsPane = ({
allowedNotificationChannels,
handleChange, // Formik or custom handler
onRequestPhoneVerificationCode,
onSendPhoneVerificationCode,
phoneFormatOptions,
Expand All @@ -70,19 +71,19 @@ const NotificationPrefsPane = ({
</legend>
<ListGroup>
{allowedNotificationChannels.map((type) => {
// TODO: If removing the Save/Cancel buttons on the account screen,
// persist changes immediately when onChange is triggered.
const inputId = `notification-channel-${type}`
const inputDescriptionId = `${inputId}-description`
return (
<NotificationOption key={type}>
<span>
<Field
aria-describedby={inputDescriptionId}
// TODO: Check this condition.
disabled={type === 'push' && !pushDevices}
id={inputId}
name="notificationChannel"
// Override onChange explicitly to use the custom one for existing accounts.
// (The Formik's one will still be used for new accounts.)
onChange={handleChange}
type="checkbox"
value={type}
/>
Expand All @@ -105,8 +106,12 @@ const NotificationPrefsPane = ({
) : (
<span id={inputDescriptionId}>
{pushDevices ? (
// TODO: i18n
`${pushDevices} devices registered`
<FormattedMessage
id="components.NotificationPrefsPane.devicesRegistered"
values={{
count: pushDevices
}}
/>
) : (
<FormattedMessage id="components.NotificationPrefsPane.noDeviceForPush" />
)}
Expand Down
7 changes: 5 additions & 2 deletions lib/components/user/sequential-pane-display.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export interface PaneProps {

interface OwnProps {
activePaneId: string
onFinish?: () => void
panes: PaneProps[]
}

Expand Down Expand Up @@ -57,7 +58,7 @@ class SequentialPaneDisplay<T> extends Component<Props<T>> {
}

_handleToNextPane = async (e: MouseEvent<Button>) => {
const { activePane, activePaneIndex, panes } = this.props
const { activePane, activePaneIndex, onFinish, panes } = this.props
const { invalid, invalidMessage } = activePane

if (activePaneIndex < panes.length - 1) {
Expand All @@ -75,8 +76,10 @@ class SequentialPaneDisplay<T> extends Component<Props<T>> {
}
this._routeTo(nextId)
}
this._focusHeader()
} else if (onFinish) {
onFinish()
}
this._focusHeader()
}

_handleToPrevPane = () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,21 @@ import React, { useEffect, useState } from 'react'

import { InlineLoading } from '../narrative/loading'

import { PageHeading, StackedPaneContainer } from './styled'
import FormNavigationButtons from './form-navigation-buttons'
import StackedPanes, { Props as StackedPanesProps } from './stacked-panes'

type Props = {
interface Props extends StackedPanesProps {
onCancel: () => void
paneSequence: any[]
title?: string | JSX.Element
}

/**
* This component handles the flow between screens for new OTP user accounts.
*
* TODO: add types once Pane type exists
*/
const StackedPaneDisplay = ({
const StackedPanesWithSave = ({
onCancel,
paneSequence,
panes,
title
}: Props): JSX.Element => {
// Create indicator of if cancel button was clicked so that child components can know
Expand All @@ -28,22 +26,11 @@ const StackedPaneDisplay = ({

useEffect(() => {
setButtonClicked('')
}, [paneSequence])
}, [panes])

return (
<>
{title && <PageHeading>{title}</PageHeading>}
{paneSequence.map(
({ hidden, pane: Pane, props, title }, index) =>
!hidden && (
<StackedPaneContainer key={index}>
<h3>{title}</h3>
<div>
<Pane canceled={isBeingCanceled} {...props} />
</div>
</StackedPaneContainer>
)
)}
<StackedPanes canceling={isBeingCanceled} panes={panes} title={title} />

<FormNavigationButtons
backButton={{
Expand Down Expand Up @@ -78,4 +65,4 @@ const StackedPaneDisplay = ({
</>
)
}
export default StackedPaneDisplay
export default StackedPanesWithSave
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we do this instead of adding a prop? Is it formik nonsense?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's to separate the code noise brought by handling the navigation button states.

Loading
Loading