Remove save button in user settings #1030

binh-dam-ibigroup · 2023-10-12T17:03:11Z

Description

This PR removes the save button form user settings, so that all settings behave consistently.

Upon changes of notification settings, accessibility, and trip history, settings are saved immediately and a toast notification is shown as confirmation.

PR Checklist:

Does the code follow accessibility standards (WCAG 2.1 AA Compliant)?
Are all languages supported (Internationalization/Localization)?
Are appropriate Typescript types implemented?

…ing account.

… change.

…a update submission.

binh-dam-ibigroup · 2023-10-16T14:49:50Z

Taking this back to do some other improvements.

AdrianaCeric

I like the implementation! It's a lot more smooth without the save button. One comment and a question

AdrianaCeric · 2023-10-17T14:26:30Z

lib/actions/ui.js

@@ -51,7 +51,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


Suggested change

* @param {string} url path to route to

* @param {string} url path to route to

AdrianaCeric · 2023-10-19T14:46:17Z

lib/components/user/new-account-wizard.tsx

    )
  }

-  const { hasConsentedToTerms, notificationChannel = 'email' } = userData
+  const { hasConsentedToTerms, notificationChannel = ['email'] } = userData


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

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

AdrianaCeric · 2023-10-19T15:42:03Z

lib/components/util/toasts.tsx

+/**
+ * Helper for displaying formatted toasts.
+ */
+export function toastSuccess(title: string, description: string): void {


Could this toast helper function also release a toast.error if the form fails to save?

Actually, an alert is shown if an error occurred!

I was thinking that having the same styling for an error and a success would ensure visual consistency

Wait no I'm not a huge fan of this that might be confusing

The toasts don't steal your attention enough

That's why in #992 I only applied toasts to confirmation messages and not alerts.

…able input if error Also apply PR feedback.

miles-grant-ibigroup

The is clearly well thought out, tested, and planned. However, there is one behavior that feels weird: can we disable the entire form during save? Otherwise it's confusing as to why the one item gets disabled. I think the other alternative would be to transform the checkbox into a spinner or something (this could be done with pure css) but as is I think it's a bit confusing.

Also, could we add a TODO to style the toasts a bit? They're a bit out of place with the rest of the UI

miles-grant-ibigroup · 2023-10-23T19:02:06Z

lib/components/user/stacked-panes-with-save.tsx

@@ -78,4 +65,4 @@ const StackedPaneDisplay = ({
    </>
  )
 }
-export default StackedPaneDisplay
+export default StackedPanesWithSave


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

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

binh-dam-ibigroup · 2023-10-23T21:14:28Z

I think the other alternative would be to transform the checkbox into a spinner or something (this could be done with pure css)

How about putting a cursor: wait style, with an aria-status "updating" while a single update is occurring?

binh-dam-ibigroup · 2023-10-23T21:15:48Z

Also, could we add a TODO to style the toasts a bit? They're a bit out of place with the rest of the UI

I agree that toast styling could generally be improved, but I won't add a TODO for this PR.

miles-grant-ibigroup · 2023-10-24T12:48:25Z

I think the other alternative would be to transform the checkbox into a spinner or something (this could be done with pure css)

How about putting a cursor: wait style, with an aria-status "updating" while a single update is occurring?

Let's give it a shot!

…ogress.

…ers.

…ate request is pending.

binh-dam-ibigroup · 2023-10-24T19:05:12Z

I think the other alternative would be to transform the checkbox into a spinner or something (this could be done with pure css)

How about putting a cursor: wait style, with an aria-status "updating" while a single update is occurring?

Let's give it a shot!

See latest commits.

miles-grant-ibigroup

New effect looks cool! is there any way we can get the loading cursor to appear on the entire label instead of just the checkbox?

… input.

binh-dam-ibigroup · 2023-10-25T15:04:42Z

New effect looks cool! is there any way we can get the loading cursor to appear on the entire label instead of just the checkbox?

Yes, see b188043.

miles-grant-ibigroup

I would've done this with classes but this seems good enough for now! Excited for these changes to move forward it's much cleaner

binh-dam-ibigroup added 13 commits October 9, 2023 16:48

refactor(stacked-panes): Separate layout code from save/cancel buttons.

e412ed8

refactor(stacked-panes): Add more types to component.

c93a6ca

refactor(existing-account-display): Submit changes right away.

2874d67

refactor(existing-account-display): Fix types

ae94f94

refactor(notification-prefs-pane): Set onChange handler explicitly.

1af40ce

refactor(notification-prefs-pane): Add missing i18n

263c940

refactor(user-account-screen): Remove form element when viewing exist…

9a5ffc7

…ing account.

refactor(user-account-screen): Remove unused validation.

06b9bc6

improvement(existing-account-display): Display toast to confirm field…

9783e83

… change.

style(existing-account-display): Clean up code.

9904996

refactor(user-account-screen): Convert to TypeScript

57a9b68

Merge branch 'dev' into remove-user-setting-save-btn

f482067

chore(i18n): Add i18n group exceptions.

15cd0be

binh-dam-ibigroup requested a review from AdrianaCeric October 13, 2023 17:44

binh-dam-ibigroup assigned AdrianaCeric Oct 13, 2023

improvement(existing-account-display): Disable inputs during user dat…

760539b

…a update submission.

binh-dam-ibigroup assigned binh-dam-ibigroup and unassigned AdrianaCeric Oct 16, 2023

binh-dam-ibigroup removed the request for review from AdrianaCeric October 16, 2023 14:50

binh-dam-ibigroup added 2 commits October 16, 2023 17:07

improvement(user-account-screen): Consolidate input change handling.

fafbe96

refactor(user-account-screen): Fix types.

2052b17

binh-dam-ibigroup requested a review from AdrianaCeric October 17, 2023 13:16

binh-dam-ibigroup assigned AdrianaCeric and unassigned binh-dam-ibigroup Oct 17, 2023

AdrianaCeric approved these changes Oct 19, 2023

View reviewed changes

AdrianaCeric assigned binh-dam-ibigroup and unassigned AdrianaCeric Oct 19, 2023

binh-dam-ibigroup added 2 commits October 19, 2023 17:06

improvement(user-account-screen): Add i18n to profile upd error, reen…

64f98db

…able input if error Also apply PR feedback.

Merge branch 'dev' into remove-user-setting-save-btn

c9407f7

binh-dam-ibigroup removed their assignment Oct 19, 2023

miles-grant-ibigroup requested changes Oct 23, 2023

View reviewed changes

miles-grant-ibigroup assigned binh-dam-ibigroup and unassigned miles-grant-ibigroup Oct 23, 2023

binh-dam-ibigroup added 4 commits October 24, 2023 10:12

Merge branch 'dev' into remove-user-setting-save-btn

86bd255

improvement(user-account-screen): Add visuals to denote submission pr…

9487b54

…ogress.

improvement(user-account-screen): Apply animation styles to all brows…

70b8881

…ers.

improvement(user-account-screen): Add "updating" toast while user upd…

0c33524

…ate request is pending.

binh-dam-ibigroup requested a review from miles-grant-ibigroup October 24, 2023 19:05

binh-dam-ibigroup assigned miles-grant-ibigroup and unassigned binh-dam-ibigroup Oct 24, 2023

refactor(user-account-screen): Tweak types

eb85c70

miles-grant-ibigroup requested changes Oct 25, 2023

View reviewed changes

miles-grant-ibigroup assigned binh-dam-ibigroup and unassigned miles-grant-ibigroup Oct 25, 2023

improvement(user-account-screen): Add wait cursor to label of changed…

b188043

… input.

binh-dam-ibigroup requested a review from miles-grant-ibigroup October 25, 2023 15:04

binh-dam-ibigroup assigned miles-grant-ibigroup and unassigned binh-dam-ibigroup Oct 25, 2023

miles-grant-ibigroup approved these changes Oct 25, 2023

View reviewed changes

miles-grant-ibigroup assigned binh-dam-ibigroup and unassigned miles-grant-ibigroup Oct 25, 2023

Merge branch 'dev' into remove-user-setting-save-btn

18f430b

binh-dam-ibigroup merged commit b9976a9 into dev Oct 26, 2023
9 checks passed

binh-dam-ibigroup deleted the remove-user-setting-save-btn branch October 26, 2023 16:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove save button in user settings #1030

Remove save button in user settings #1030

binh-dam-ibigroup commented Oct 12, 2023 •

edited

Loading

binh-dam-ibigroup commented Oct 16, 2023

AdrianaCeric left a comment

AdrianaCeric Oct 17, 2023

AdrianaCeric Oct 19, 2023

binh-dam-ibigroup Oct 19, 2023

AdrianaCeric Oct 19, 2023

binh-dam-ibigroup Oct 19, 2023

AdrianaCeric Oct 23, 2023

miles-grant-ibigroup Oct 24, 2023

AdrianaCeric Oct 24, 2023

miles-grant-ibigroup Oct 24, 2023

binh-dam-ibigroup Oct 24, 2023

miles-grant-ibigroup left a comment

miles-grant-ibigroup Oct 23, 2023

binh-dam-ibigroup Oct 24, 2023

binh-dam-ibigroup commented Oct 23, 2023

binh-dam-ibigroup commented Oct 23, 2023

miles-grant-ibigroup commented Oct 24, 2023

binh-dam-ibigroup commented Oct 24, 2023

miles-grant-ibigroup left a comment

binh-dam-ibigroup commented Oct 25, 2023

miles-grant-ibigroup left a comment

	* @param {string} url path to route to
	* @param {string} url path to route to

Remove save button in user settings #1030

Remove save button in user settings #1030

Conversation

binh-dam-ibigroup commented Oct 12, 2023 • edited Loading

Description

binh-dam-ibigroup commented Oct 16, 2023

AdrianaCeric left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

miles-grant-ibigroup left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

binh-dam-ibigroup commented Oct 23, 2023

binh-dam-ibigroup commented Oct 23, 2023

miles-grant-ibigroup commented Oct 24, 2023

binh-dam-ibigroup commented Oct 24, 2023

miles-grant-ibigroup left a comment

Choose a reason for hiding this comment

binh-dam-ibigroup commented Oct 25, 2023

miles-grant-ibigroup left a comment

Choose a reason for hiding this comment

binh-dam-ibigroup commented Oct 12, 2023 •

edited

Loading