FEATURE: Incorporate PWA install prompt into Discourse UI (PR #8013)

This is mainly done so Discourse forums stop nagging people to install on the very first visits to a website.

We will prevent the native install “mini-info” bar from ever appearing, capture the event that pops with it, and delay it until the user meets our criteria, which currently is trust_level 1.

If the event happens and the user meets our criteria we show a Discourse alert banner proposing the install to the user. Dismissal of the banner is recorded so the user ins’t bothered anymore on the same device.

GitHub

You’ve signed the CLA, xfalcox. Thank you! This pull request is ready for review.

why do you need that here ?

can probably be replaced by:

this.get("currentUser.trust_level") > 0

Can we wrap {{i18n 'pwa.install_banner.question'}} in a span please? I don’t like text with no direct html parent.

Can we wrap {{i18n 'pwa.install_banner.question'}} in a span please? I don’t like text with no direct html parent.

Is this event listener being removed somewhere ? I’m afraid it’s leaking.

Also you are setting a property to an even, which probably holds a reference to a DOM object and is probably going to leak too. We should ensure this is unset on destruction of the component.

I think this comment is not necessary. this.currentUser.trust_level > 0 seems very clear that we are checking we are above trust_level 1

I think this comment is not necessary. this.currentUser.trust_level > 0 seems very clear that we are checking we are above trust_level 1

This comment is also not necessary IMO

This comment is also not necessary IMO

Usually this kind of const should be UPPERCASE: USER_DISMISSED_PROMPT_KEY

Should be turnOn no ?

Just so I can set on line 18. Is there a better way?

Just so I can set on line 18. Is there a better way?

  window.addEventListener("beforeinstallprompt", e => {
      // Prevent Chrome 76+ from automatically showing the prompt
      e.preventDefault();
      // Stash the event so it can be triggered later
      this.set("deferredInstallPromptEvent", e);
    });
  }

Should just work as your are using fat arrow. It doesn’t?

  window.addEventListener("beforeinstallprompt", e => {
      // Prevent Chrome 76+ from automatically showing the prompt
      e.preventDefault();
      // Stash the event so it can be triggered later
      this.set("deferredInstallPromptEvent", e);
    });
  }

Should just work as your are using fat arrow. It doesn’t?

There’s no need to concatenate strings here. We have a component for this:

        {{discourse-linked-text action=(action "turnOn") translatedText=(i18n "pwa.install_banner" title=siteSettings.title)}}</a>
1 Like

You can remove the question and link strings and use this instead:

      install_banner: "Do you want to <a href>install %{title} on this device?</a>"

It would be better I think to use the KeyValueStore for this. It has some extra checks about if localStorage works.

1 Like