Skip to content

Changes to toast promise #1455

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 11 commits into from
Aug 27, 2025
Merged

Changes to toast promise #1455

merged 11 commits into from
Aug 27, 2025

Conversation

@amy-corson-ibigroup
Copy link
Contributor

@amy-corson-ibigroup amy-corson-ibigroup commented Aug 18, 2025 •
edited
Loading

Description:

  • Fixes a bug wherein if a user is not logged in and attempts to save a place as home or work, a loading toast will pop up that cannot be dismissed.
  • Adds a notification to announce when a saved trip has been deleted.
  • Also removes the userSuccess number result from rememberPlace as it wasn't being used anywhere.

PR Checklist:

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

josh-willis-arcadis
josh-willis-arcadis approved these changes Aug 21, 2025
View reviewed changes
Copy link
Contributor

@josh-willis-arcadis josh-willis-arcadis left a comment

Choose a reason for hiding this comment

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

great changes!

@daniel-heppner-ibigroup daniel-heppner-ibigroup mentioned this pull request Aug 26, 2025
Merged
daniel-heppner-ibigroup
daniel-heppner-ibigroup requested changes Aug 26, 2025
View reviewed changes
Copy link
Contributor

@daniel-heppner-ibigroup daniel-heppner-ibigroup left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Take a look at my piggyback PR. If you merge that then this is good to go :) #1460

lib/components/util/toasts.tsx Outdated
id?: string,
silentOnSuccess = false
): Promise<any> => {
if (!promise) return
Copy link
Contributor

Choose a reason for hiding this comment

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

Is promise optional? Can you mark it as optional in the argument list?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm I dont think this should be optional. This line is here because in the case of rememberPlace, if a user isn't logged in, the promise gets passed into this function as undefined

Copy link
Contributor

Choose a reason for hiding this comment

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

If it can be undefined then it should be optional! There should be a type error somewhere else if there's an undefined that gets passed into this function, since undefined is not compatible with Promise. If there's no type error is probably because there's an any type somewhere else that maybe should be fixed as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right! Reworked this in 1fbe9bd, i really don't think the promise should ever be undefined

daniel-heppner-ibigroup
daniel-heppner-ibigroup approved these changes Aug 27, 2025
View reviewed changes
Copy link
Contributor

@daniel-heppner-ibigroup daniel-heppner-ibigroup left a comment

Choose a reason for hiding this comment

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

Looks great to me! Thanks for the improvements!

lib/components/util/toasts.tsx Outdated

export const toastPromise = async (
promise: Promise<any>,
export const toastPromise = async <T,>(
Copy link
Contributor

Choose a reason for hiding this comment

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

cool!

@amy-corson-ibigroup amy-corson-ibigroup merged commit 158afa2 into dev Aug 27, 2025
9 checks passed
@amy-corson-ibigroup amy-corson-ibigroup deleted the changes-to-toast-promise branch August 27, 2025 18:56
@amy-corson-ibigroup amy-corson-ibigroup mentioned this pull request Aug 27, 2025
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@daniel-heppner-ibigroup daniel-heppner-ibigroup daniel-heppner-ibigroup approved these changes

@josh-willis-arcadis josh-willis-arcadis josh-willis-arcadis approved these changes

Assignees

@amy-corson-ibigroup amy-corson-ibigroup

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@amy-corson-ibigroup @daniel-heppner-ibigroup @josh-willis-arcadis @danielhep