-
Notifications
You must be signed in to change notification settings - Fork 942
809 dont show toast notifications on ftc #22667
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
Conversation
Reduces complexity
We need to implement error handling here, but for now, lets no show the opt in as if it is saved in db
16c25ae to
568aeeb
Compare
Fix tests
568aeeb to
34ddcca
Compare
Pull Request Test Coverage Report for Build 1b9c31ce77767849a63850005ed3bedaa6d28813Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
e296707 to
2f8db84
Compare
2f8db84 to
47636e5
Compare
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.
CR: ✅
Acc: 🔴
I feel this behavior is not what we want:
- I'm on the FTC tab
- I delete the option from the db
- I refresh the FTC tab -> I don't see the notification ✅
- I switch to one of the other two General tabs -> I see the notification ✅
- I refresh the tab -> I still get the notification ❌
- If I switch to the FTC tab and Go back to one of the other two tabs, the notification is not shown ✅
mounting will update the db. Unmounting and dismissing will hide the opt in.
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.
CR && Acc: ✅
Context
Summary
This PR can be summarized in the following changelog entry:
Relevant technical choices:
Test instructions
Test instructions for the acceptance test before the PR gets merged
This PR can be acceptance tested by following these steps:
wp_usermeta, search forwpseo_seen_llm_txt_opt_in_notificationand remove it.wp_usermeta, search forwpseo_seen_llm_txt_opt_in_notificationand remove it.wp_usermeta, search forwpseo_seen_llm_txt_opt_in_notificationand remove it.Relevant test scenarios
Test instructions for QA when the code is in the RC
QA can test this PR by following these steps:
Impact check
This PR affects the following parts of the plugin, which may require extra testing:
Other environments
[shopify-seo], added test instructions for Shopify and attached theShopifylabel to this PR.Documentation
Quality assurance
Innovation
innovationlabel.Fixes Don't show toast notifications on FTC