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

Update Feedback Question banner #18308

Merged
merged 9 commits into from Nov 15, 2021
Merged

Update Feedback Question banner #18308

merged 9 commits into from Nov 15, 2021

Conversation

peterhashair
Copy link
Contributor

@peterhashair peterhashair commented Nov 14, 2021

Description:

Update feedback Question Banner, Fix random UI failed for UIIntegrationTest I think this is related to #18124, update selection.json missing heart icon #18262.

Also, follow up on request here #18251 (comment)

  • Hide question banner on mobile
  • add event hook for banner
  • update event show condition

Review

Peter Zhang added 5 commits November 15, 2021 10:17
update table random UI
update icon list
update screen shots
hide feedback on mobile
update feed back
@peterhashair peterhashair changed the title Fix random UI failed, update selection.json Update Feedback Question Nov 15, 2021
@peterhashair peterhashair changed the title Update Feedback Question Update Feedback Question banner Nov 15, 2021
update feedback Test
@peterhashair peterhashair linked an issue Nov 15, 2021 that may be closed by this pull request
21 tasks
@peterhashair peterhashair added not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. Needs Review PRs that need a code review labels Nov 15, 2021
Peter Zhang added 3 commits November 15, 2021 16:22
update UI screenshot Test
update UI tests
update screenshots
@sgiehl sgiehl added this to the 4.6.0 milestone Nov 15, 2021
Comment on lines -124 to +132
if ($nextReminderDate === false || $nextReminderDate <= 0) {

// if user is created more than 6 month ago, set reminder to today and show banner
$userCreatedDate = Piwik::getCurrentUserCreationData();
if (!empty($userCreatedDate) && Date::factory($userCreatedDate)->addMonth(6)->getTimestamp() < $now) {
$nextReminder = Date::now()->getStartOfDay()->subDay(1)->toString('Y-m-d');
$feedbackReminder->setUserOption($nextReminder);
return true;
}
if ($nextReminderDate === false) {
Copy link
Member

Choose a reason for hiding this comment

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

This actually makes it worse, as everyone will now get notified 6 months after the update. I will merge this PR now nevertheless, so the other fixes are in 4.x-dev. Will create a PR to improve those checks myself later

@sgiehl sgiehl merged commit 9db3daa into 4.x-dev Nov 15, 2021
@sgiehl sgiehl deleted the fix-random-ui-failed branch November 15, 2021 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Collect feedback about what features people are using?
2 participants