-
Notifications
You must be signed in to change notification settings - Fork 34
trigger product update modal #459
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
trigger product update modal #459
Conversation
|
@droid-mohit @jayeshsadhwani99 Thank you for your patience and help. Created the new PR here. Apologies for the confusion with the previous PR once again. |
|
@jayeshsadhwani99 can you please review this PR? |
|
@rishsanyal Please update your branch from |
d749b0c to
707ffc8
Compare
jayeshsadhwani99
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The expected behaviour is:
- We click on Yes, the yes handler, or in your case the single handler works
- We click on No, similar behaviour
- When we click outside or click on the close button on the top right, It has the same behaviour as no.
In your case, yes and no work fine but when clicked outside the modal or clicked on cross, the same bug still arises.
There are 2 solutions I see:
- You can add the same event as in no and do it that way (simpler in this case)
- You can refresh the page whenever there is an event on this modal. Since everything is stored in localstorage should work without any problems since the state will be refreshed
You can do any of the above or if you have a better solution you can also do that
|
@jayeshsadhwani99 : Thank you for catching that. I've updated the code to register the event as the user not signing up for updates now. |
jayeshsadhwani99
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same issue occurs when the user clicks on the close icon, please change line 35 too
There's a small issue for first-time users where every page will show the Product Update Modal to the customer.
Playbooks currently depends on setting the last-login time cookie to know if this modal should be displayed or not. There's 2 issues with this:
New customers from the same browser won't get the chance to sign up for product updates
A first time customer will just face annoyances for the first time they interact with Playbooks' platform.
This PR fixes that by ensuring that on registering we focus on the new customer through setting the option in the redux store and clearing the last-login time cookie on re-registering
I've added 2 videos below. The first one shows the issue and the second validates the fix.
Issue:
https://github.com/user-attachments/assets/2f6c0db1-cc50-4bad-8b9c-fd38516a8887
Validated Fix:
modal_fix.mov