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
Modiy refer banner #17598
Modiy refer banner #17598
Conversation
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.
looks good @flamisz 👍 Left a comment and wondering if we can directly close the popup once a share link was clicked and then we never show it again. Assuming most people would only share through one network anyway.
I haven't looked yet is it already implemented to not show the banner again once clicked on a social network? Could maybe show in the thank you message otherwise "Thank you... we won't show this banner again" or something?
plugins/Feedback/Feedback.php
Outdated
@@ -135,7 +139,12 @@ public function showReferBanner() | |||
$nextReminderDate = $referReminder->getUserOption(); | |||
|
|||
if ($nextReminderDate === false) { | |||
return true; | |||
$nextReminder = Date::now()->getStartOfDay()->addDay(6 * 30)->toString('Y-m-d'); |
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.
@flamisz I believe the "review reminder" is showing every 90 days so there's a risk both review and refer banners and shown at same point.
Maybe after 6 months (180 days) we might show both reminders approx on a similar day? The 90 days in a review reminder is sent from plugins/Feedback/angularjs/feedback-popup/feedback-popup.controller.js
. I now wonder if we maybe could add 135 days to the next "review reminder" date? This way it be 45 days before the popup would be shown. If no review reminder date is set yet, then we would maybe need to set the review reminder to be in 90 days and the refer reminder to be in 135 days?
Bit complicated just wanting to avoid them being shown to close of each other :)
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.
Actually I check in the code, at line 111:
if ($this->getShouldPromptForFeedback()) {
...
This way we can't show it at the same time.
But to make sure they don't come up close to each other (next day, etc), make sense. I'll check your recommendation and add it.
We can do that, yes. I think originally we tried something like this but modified thinking about someone who wants to share on multiple platforms. So I will modify it to close the modal, save it to not show it again, and show the thanks message with the recommended text. |
Cheers @flamisz 💯 |
Description:
fixes #17571
Review