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

Fix/is 276 toast rerender #1333

Merged
merged 2 commits into from
Jul 5, 2023
Merged

Conversation

alexanderleegs
Copy link
Contributor

This PR fixes an issue with multiple rerenders of the toast components - the behaviour of react-hook-form and react-query seem to have changed after our dependencies update, and this caused issues for all of our toasts which were contained within useEffect, as the component would get rerendered, which changed the reference to toast and triggered the useEffect again.

The solution to this issue is to modify the toasts to use the id param - this prevents duplicate toasts from appearing (link). The components which make use of toasts have been modified to support this.

Also resolves IS-284 once the privatisation PR has been rebased onto this one.

@alexanderleegs alexanderleegs requested a review from a team July 5, 2023 01:35
@alexanderleegs alexanderleegs temporarily deployed to staging July 5, 2023 01:46 — with GitHub Actions Inactive
@alexanderleegs alexanderleegs temporarily deployed to staging July 5, 2023 01:50 — with GitHub Actions Inactive
@@ -12,7 +12,7 @@ const retrieveThirdNavOptions = async (
collectionName,
isExistingCollection
) => {
const errorToast = useErrorToast
const errorToast = useErrorToast()
Copy link
Contributor

Choose a reason for hiding this comment

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

huh so this never worked :sadge:

export const useErrorToast = (): UseToastReturn =>
useToast({
// Not specifying return type because type is in the argument
const useIsomerToast = (args: UseToastProps) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

wah I think we need to define a return type here, but fell into a rabbit hole for this cuz we got a version mismatch between chakra and design system.
specifically, danger doesnt exist anymore, should be error now, but design system is lagging behind, actually not sure if this is already a bug in prod :sadge:

Rather than putting this, could you create a ticket and assign it to me? is related to the design system changes

@alexanderleegs alexanderleegs merged commit cb03b07 into develop Jul 5, 2023
@mergify mergify bot deleted the fix/is-276-toast-rerender branch July 5, 2023 04:21
@mergify mergify bot requested a review from a team August 4, 2023 04:22
@mergify
Copy link

mergify bot commented Aug 4, 2023

This pull request has been stale for more than 30 days! Could someone please take a look at it @isomerpages/iso-engineers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants