wp-media / imagify-plugin

Speed up your website with lighter images without losing quality.
https://imagify.io
69 stars 24 forks source link

Closes #728 Replace out of quota notice by upsell block #752

Closed Tabrisrp closed 7 months ago

Tabrisrp commented 8 months ago

Description

Display an upsell banner and offer users who are out of the quota to upgrade to a higher plan.

Remove the banner at the top, we don’t need both banners since the same information would be displayed.

Fixes #728

Type of change

Is the solution different from the one proposed during the grooming?

No

Checklists

Generic development checklist

Test summary

vmanthos commented 7 months ago

@Tabrisrp The icon /assets/images/stormy.svg that should be displayed in the banner isn't visible: image

It should be: image

This is because it is of the same color as the background.

Using the filter property can resolve this. https://developer.mozilla.org/en-US/docs/Web/CSS/filter#browser_compatibility

Can you please look into this?

vmanthos commented 7 months ago

@Tabrisrp I got the following PHP fatal error:

PHP Fatal error:  Uncaught TypeError: call_user_func_array(): Argument #1 ($callback) must be a valid callback, class Imagify\Notices\Notices does not have a method "renew_almost_over_quota_notice" in /var/www/vasilis.rocketlabsqa.ovh/htdocs/wp-includes/class-wp-hook.php:312
Stack trace:
#0 /var/www/vasilis.rocketlabsqa.ovh/htdocs/wp-includes/class-wp-hook.php(334): WP_Hook->apply_filters()
#1 /var/www/vasilis.rocketlabsqa.ovh/htdocs/wp-includes/plugin.php(517): WP_Hook->do_action()
#2 /var/www/vasilis.rocketlabsqa.ovh/htdocs/wp-content/plugins/imagify/classes/User/User.php(221): do_action()
#3 /var/www/vasilis.rocketlabsqa.ovh/htdocs/wp-content/plugins/imagify/classes/User/User.php(241): Imagify\User\User->get_percent_consumed_quota()
#4 /var/www/vasilis.rocketlabsqa.ovh/htdocs/wp-content/plugins/imagify/views/part-upsell.php(8): Imagify\User\User->get_percent_unconsumed_quota()
#5 /var/www/vasilis.rocketlabsqa.ovh/htdocs/wp-content/plugins/imagify/inc/classes/class-imagify-views.php(590): include('...')
#6 /var/www/vasilis.rocketlabsqa.ovh/htdocs/wp-content/plugins/imagify/inc/classes/class-imagify-views.php(605): Imagify_Views->get_template()
#7 /var/www/vasilis.rocketlabsqa.ovh/htdocs/wp-content/plugins/imagify/views/part-settings-account.php(33): Imagify_Views->print_template()
#8 /var/www/vasilis.rocketlabsqa.ovh/htdocs/wp-content/plugins/imagify/inc/classes/class-imagify-views.php(590): include('...')
#9 /var/www/vasilis.rocketlabsqa.ovh/htdocs/wp-content/plugins/imagify/inc/classes/class-imagify-views.php(605): Imagify_Views->get_template()
#10 /var/www/vasilis.rocketlabsqa.ovh/htdocs/wp-content/plugins/imagify/views/page-settings.php(87): Imagify_Views->print_template()
#11 /var/www/vasilis.rocketlabsqa.ovh/htdocs/wp-content/plugins/imagify/inc/classes/class-imagify-views.php(590): include('...')
#12 /var/www/vasilis.rocketlabsqa.ovh/htdocs/wp-content/plugins/imagify/inc/classes/class-imagify-views.php(605): Imagify_Views->get_template()
#13 /var/www/vasilis.rocketlabsqa.ovh/htdocs/wp-content/plugins/imagify/inc/classes/class-imagify-views.php(242): Imagify_Views->print_template()
#14 /var/www/vasilis.rocketlabsqa.ovh/htdocs/wp-includes/class-wp-hook.php(310): Imagify_Views->display_settings_page()
#15 /var/www/vasilis.rocketlabsqa.ovh/htdocs/wp-includes/class-wp-hook.php(334): WP_Hook->apply_filters()
#16 /var/www/vasilis.rocketlabsqa.ovh/htdocs/wp-includes/plugin.php(517): WP_Hook->do_action()
#17 /var/www/vasilis.rocketlabsqa.ovh/htdocs/wp-admin/admin.php(259): do_action()
#18 /var/www/vasilis.rocketlabsqa.ovh/htdocs/wp-admin/options-general.php(10): require_once('...')
#19 {main}
  thrown in /var/www/vasilis.rocketlabsqa.ovh/htdocs/wp-includes/class-wp-hook.php on line 312

It seems that the renew_almost_over_quota_notice() method isn't there anymore.

If that's the way it should be, the following should be removed as well: https://github.com/wp-media/imagify-plugin/blob/1ad55b5341f04291b92c25c3aedc438c607aa149/classes/Notices/Notices.php#L137

vmanthos commented 7 months ago

@Tabrisrp There is another fatal error coming from here: https://github.com/wp-media/imagify-plugin/blob/1ad55b5341f04291b92c25c3aedc438c607aa149/views/part-upsell.php#L65

esc_esc_html__html_e() is not defined.

Can you please look into this as well? :pray:

vmanthos commented 7 months ago

@Tabrisrp Thank you for the fixes.

I don't see the implementation of the Black Friday promotion where text and URLs for upgrades should differ from the ones usually used.

Should this be part of the specific PR?

Tabrisrp commented 7 months ago

@vmanthos In the acceptance criteria, Piotr specified that the promo changes are not to be included in this PR

vmanthos commented 7 months ago

Thank you for checking, @Tabrisrp!

That was in bold, but still I managed to miss it. :sweat_smile:

vmanthos commented 7 months ago

@Tabrisrp @wp-media/productimagify The notice is correctly displayed and it's dismissable. However, I don't see a way for that to be displayed again to the same user except for uninstalling the plugin.

According to the specs doc:

The banner should be dismissable (and once it’s dismissed it shouldn’t be displayed until the user is out of the quota again)

Currently, the banner is displayed when the unconsumed quota is less than 20% of the plan's quota. If the banner is dismissed and more quota is used the banner isn't displayed.

Should it be when the unconsumed quota is 0%, or anything different? :pray:

Tabrisrp commented 7 months ago

The dismiss is reset if the quota gets back above 20%, and the banner should show again after that when it gets below 20%.