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

X2-8266 | Cannot Decline Reservation through the Notifications Tab #295

Closed
wants to merge 4 commits into from

Conversation

VitalyDevico
Copy link
Contributor

No description provided.

@VitalyDevico VitalyDevico reopened this Dec 18, 2023
Comment on lines 41 to 45
if (shouldCloseOnRouteChange && location?.search?.includes("redirect=true")) {
setIsLeftDrawerOpen(false);
setIsRightDrawerOpen(false);
}
}, [location.search, shouldCloseOnRouteChange]);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think its better to close the drawer from x2-seller app. since this ui-kit component might be used across different apps. can you try to trigger the close event, when we click on decline / accept purchase

Also, to prevent closing this drawer on notifications changes, we can pass additional props shouldCloseOnChanges - pass this as false from x2-seller app.

@rushi can you also review this once

Copy link
Contributor Author

@VitalyDevico VitalyDevico Dec 19, 2023

Choose a reason for hiding this comment

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

done, could you please take another look
also, since we are preventing closing on notification changes, we need to manually close it after opening the purchase, to handle the case which was fixed by adding close-on-change logic previously (when we open a purchase from the purchases page), this logic added as part of the x2-seller PR https://github.com/xola/x2-seller/pull/3773

Comment on lines +34 to +35
isRightDrawerVisible = null,
setIsRightDrawerVisible,
Copy link
Member

Choose a reason for hiding this comment

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

Why do you need these two? There's too many props coming in.

Copy link
Member

@rushi rushi Dec 20, 2023

Choose a reason for hiding this comment

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

12 props 😱

Copy link
Contributor Author

Choose a reason for hiding this comment

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

in order to trigger closing of drawer from the seller app as proposed in comment above

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was thinking on other way to fixing this via url params, maybe you have some suggestions how to optimize? in the end we need to handle case when the api call is made on the background and not close the drawer, but another case when we need to close it when purchase tile clicked while the user on the /purchases route

@rushi rushi closed this Jan 8, 2024
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