Automattic / newspack-plugin

An advanced open-source publishing and revenue-generating platform for news organizations.
https://newspack.com
GNU General Public License v2.0
325 stars 48 forks source link

Plugin OneSignal to push notifications #1510

Open hhugosz opened 2 years ago

hhugosz commented 2 years ago

Hey guys. As much as the One Signal plugin appears on the approved plugins list, it looks like it needs some special configuration. My site is hosted on Wordpress.Com and is all AMP. Can you help me to implement the plugin?

yogeshbeniwal commented 2 years ago

AMP compatibility is a work in progress by @adekbadek at https://github.com/Automattic/newspack-plugin/pull/1076. Last update was half year back.

adekbadek commented 2 years ago

Hi @hhugosz, this should work in theory – as a plugin file (e.g. wp-content/plugins/newspack-onesignal.php) – but I haven't tested:

<?php
/**
 * Plugin Name: Newspack AMP Plus OneSignal
 */

add_filter( 'newspack_amp_plus_sanitized', function( $is_sanitized, $error ){
    if ( isset( $error, $error['type'] ) && 'js_error' === $error['type'] ) {
        if (
            // OneSignal inline script.
            ( isset( $error['text'] ) && stripos( $error['text'], 'window.OneSignal' ) ) ||
            // OneSignal SDK.
            ( isset( $error['node_attributes'], $error['node_attributes']['src'] ) && stripos( $error['node_attributes']['src'], 'onesignal.com' ) )
        ) {
            $is_sanitized = false;
        }
    }
    return $is_sanitized;
}, 10, 2 );
yogeshbeniwal commented 2 years ago

@adekbadek Thanks you for sharing code snippets. Will this code also enable/make Prompt & Subscription Bell appear on AMP pages?

adekbadek commented 2 years ago

It should, yes – though it's been a while since I tested this.

yogeshbeniwal commented 2 years ago

AMP support for OneSignal plugin was released today. Though it may need some improvement.

adekbadek commented 2 years ago

Thank you for the update, @yogeshbeniwal ! Can you share more about why it may need improvement?

yogeshbeniwal commented 2 years ago

Few of the observations are:

jkasten2 commented 2 years ago

Prompt is not displayed, more important when using standard mode than transitional mode

The only option to prompt for notifications with AMP is amp-web-push-widget, which only supports a button on the page that must be clicked. Are you asking about the slide down prompt or the browser's native prompt here?

Click on subscription bell results in native browser prompt in new window rather than in same window

There is a AMP feature request for this. https://github.com/ampproject/amphtml/issues/34566

Still shows the subscription bell after users have subscribed, even though setting is to not show.

If you directly accept from this prompt it should be working, so there could be a OneSignal-Wordpress-Plugin bug here. However there is a known limitation where a number of other scarios don't work. https://github.com/ampproject/amphtml/issues/28717

NTG event analytics

Not sure about this one, would need more details.

yogeshbeniwal commented 2 years ago

The only option to prompt for notifications with AMP is amp-web-push-widget, which only supports a button on the page that must be clicked. Are you asking about the slide down prompt or the browser's native prompt here?

Asking about slide down prompt.

Not sure about this one, would need more details.

NTG events are specific to events implemented in newspack plugin. Reference here.