search

OneSignal / OneSignal-WordPress-Plugin

OneSignal is a free push notification service for web and mobile apps. This plugin makes it easy to integrate your website with OneSignal Push Notifications. https://onesignal.com
Other
80 stars 42 forks source link

AMP web push with Automattic/newspack-plugin + OneSignal-WordPress-Plugin #272

Closed jkasten2 closed 2 years ago

jkasten2 commented 3 years ago

Scope

Creating this issue to track and discuss how to make AMP web push compatible with Automattic/newspack-plugin and OneSignal-WordPress-Plugin.

Status

<amp-web-push> Tag Details

Thanks to @westonruter for these details Example of the amp-web-push component, configured with a custom element like so:

<amp-web-push
  layout="nodisplay"
  helper-iframe-url="https://example.com/helper-iframe.html"
  permission-dialog-url="https://example.com/permission-dialog.html"
  service-worker-url="https://example.com/service-worker.js"
></amp-web-push>

And then there are two custom elements for subscribing and unsubscribing to the notifications:

<!-- A subscription widget -->
<amp-web-push-widget
  visibility="unsubscribed"
  layout="fixed"
  width="250"
  height="80"
>
  <button on="tap:amp-web-push.subscribe">Subscribe to Notifications</button>
</amp-web-push-widget>

<!-- An unsubscription widget -->
<amp-web-push-widget
  visibility="subscribed"
  layout="fixed"
  width="250"
  height="80"
>
  <button on="tap:amp-web-push.unsubscribe">
    Unsubscribe from Notifications
  </button>
</amp-web-push-widget>

Integration TODO:

Thanks again to @westonruter for these details So for configuration with OneSignal, I think it's mainly a matter of just supplying the relevant URLs to <amp-web-push>. See an example in the Newspack plugin's yet-unmerged PR https://github.com/Automattic/newspack-plugin/pull/417 which seeks to add AMP-compatible push notifications. The amp-web-push attributes could be set accordingly:

So when amp_is_request() returns true, then that component could be rendered on the page. If false (or the function is not defined), then the plugin could continue doing what it does at present in OneSignal_Public::onesignal_header().

See also https://github.com/OneSignal/OneSignal-WordPress-Plugin/issues/258.

adekbadek commented 3 years ago

Hi @jkasten2 👋 At Newspack, we're very keep to integrate OneSignal, and handling AMP in this plugin (instead of our workaround) seems like the best way forward. Is this planned to be implemented?

rgomezp commented 2 years ago

Howdy y'all, It's been a while since we revisited this issue thread so I just wanted to check-in.

The plugin now automatically supports AMP (see PR). From the Newspack team, have you tested the latest plugin version? I believe it should work automatically but please let us know if there is other work needed to fully support the integration.

@adekbadek

claudiulodro commented 2 years ago

Thanks for checking and for adding compatibility! We have a number of people using the latest version with AMP-compatibility, and it's been working well for them so far.

If there is one feature request we can make it would be to have the AMP version respect the left/right settings from the WP Admin options screen. Currently it's hard-coded to the right corner. Totally not a big deal or hurry though. Feel free to close this issue. :)

rgomezp commented 2 years ago

Certainly! Great to hear it's working well.

I'll create a separate issue for the placement improvement.

gamebits commented 2 years ago

Thanks for opening that issue regarding bell placement! https://github.com/OneSignal/OneSignal-WordPress-Plugin/issues/291