search

mysociety / fixmystreet

This is mySociety's popular map-based reporting platform: easy to install in new countries and regions
http://fixmystreet.org/
Other
503 stars 235 forks source link

Don't position important UI elements behind Android banners #4153

Open jonkri opened 1 year ago

jonkri commented 1 year ago

Is your feature request related to a problem? Please describe.

The in-app install banner on Android, offering to install the PWA, is positioned in the bottom of the app and covers the “Back” and “Continue” buttons.

The translation banner, offering to translate the PWA, has the same issue.

Users may not notice or care about these banners, and may not be ready to accept or dismiss them. Also, users may not be aware that these banners cover these (necessary, in order to create a report) UI elements.

Describe the solution you'd like

I'm thinking that we may need to move important enough UI elements (such as the “Back” and “Continue” buttons) from the bottom of the viewport, so that they are visible even if these banners are displayed.

I'm not sure what we would want the UI to look like.

Describe alternatives you've considered

I was thinking that we could change how the in-app install banner is displayed using the beforeinstallprompt event as described in https://web.dev/customize-install/. However, I haven't found a way to do something similar with the translation banner.

Disabling the translation banner would decrease the accessibility of the app.

Additional context

FixaMinGata has replaced the Android Cordova app with the https://fixamingata.se/ web app packaged using PWABuilder. The Google Play listing is here. We are considering releasing the same app on App Store as well.

Implementing the crosshair way of reporting (used in the Cordova app) could perhaps be a possibility. (It was mentioned here).) This could perhaps be done at the same time.

dracos commented 1 year ago

Yes, a custom install as you link to could be useful here - we could put the install link into the navigation, or show our own message somehow or similar, that didn't obscure the buttons.

The translate banner - you mean your browser is set to English, so visiting the website it is offering to translate it? Presumably it doesn't normally offer translation for a Swedish website in a Swedish browser? It appears at the top for me, interestingly, over the header. There's a "Translate" option under the three dots, so I guess it is something we could think about disabling potentially.

"FixaMinGata has replaced the Android Cordova app with the https://fixamingata.se/ web app packaged using PWABuilder." - ooh, do you have the code you've used for this somewhere? We were going to be looking to do the same. Some form of offline reporting is the main functionality I think we want to implement as we know some users of our current native app do use that. And I think we will have to make some (hopefully not major) design updates in order for Apple to consider it for the App Store.

jonkri commented 1 year ago

Yes, a custom install as you link to could be useful here - we could put the install link into the navigation, or show our own message somehow or similar, that didn't obscure the buttons.

Sounds good!

The translate banner - you mean your browser is set to English, so visiting the website it is offering to translate it? Presumably it doesn't normally offer translation for a Swedish website in a Swedish browser?

Yes. I think it will only offer to translate when the browser language differs from the language of the web site.

It appears at the top for me, interestingly, over the header.

This is how it looks for me with an emulated Pixel device using API 32 (you can see the banner in the bottom of the page):

Screenshot_1667843403

There's a "Translate" option under the three dots, so I guess it is something we could think about disabling potentially.

Interesting! I have this option as well.

I'm not sure if the translation banner can be disabled without any undesirable side-effects for other (non-Chrome) translators... 🤔

I guess one solution could be to add the translate="no" attribute to the html start tag in templates/web/base/header.html, but that feels a bit like a bit of a sledgehammer approach to me.

https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/translate

I'm not sure if it matters, but I noticed that the translate attribute is not supported by Firefox on Android.

"FixaMinGata has replaced the Android Cordova app with the https://fixamingata.se/ web app packaged using PWABuilder." - ooh, do you have the code you've used for this somewhere? We were going to be looking to do the same.

PWABuilder basically doesn't require any code at all.

The only thing I can remember doing code-wise was to add this to the NGINX configuration:

    location /.well-known/assetlinks.json {
        root /var/www/beta.fixamingata.se;
    }

Our assetlinks.json (that we downloaded from Google Play Console) contains this:

[
  {
    "relation": ["delegate_permission/common.handle_all_urls"],
    "target": {
      "namespace": "android_app",
      "package_name": "se.sambruk.fixamingata",
      "sha256_cert_fingerprints":
        ["AA:C0:E5:1A:57:45:B8:7E:D4:EB:48:4C:A6:FD:96:FC:B2:BA:7D:93:11:EF:33:CD:E1:2D:6B:EF:0A:BC:7B:A8"]
    }
  }
]

Digital asset links are described here: https://developers.google.com/digital-asset-links/v1/getting-started.

In case you're curious, here are some notes I made when generating the app through the PWABuilder UI:

jonkri commented 1 year ago

A quick update on our PWA work:

We have now published the FixaMinGata PWA in Apple App Store as well. We are no longer using fixmystreet-mobile.

Apple rejected our first submission because of the app badges in the footer.

We set the following Info.plist properties:

(We removed NSMicrophoneUsageDescription since we're not using the microphone.)

We will be looking into fixing this issue next.

Also, users seem to prefer the crosshair way of positioning map pins (#4176).

jonkri commented 1 year ago

There's a "Translate" option under the three dots, so I guess it is something we could think about disabling potentially.

While there is a "Translate" option under the three dots, the three dots are not present when the PWA is running in standalone mode (which arguably provides the best UX in general). This means that there's no way to translate a standalone PWA app if <meta name="google" content="notranslate"> is used (as far as I know).

translate="no" didn't work in Chrome for Android, so I used <meta name="google" content="notranslate"> instead. See ee7c78e48a88f559796b7ccc05e2751b887b4cd0 and #4235.

Can FixMyStreet detect the user agent language and use that? 🤔

jonkri commented 1 year ago

Yes, a custom install as you link to could be useful here - we could put the install link into the navigation, or show our own message somehow or similar, that didn't obscure the buttons.

We disabled the installation prompt for now in the FixaMinGata cobrand. See 0ef657d9b253662d8445694a6daf017126c537fd and #4235. An in-app installation flow like the one you described would of course be even better.

jonkri commented 1 month ago

We've been using the following code in web/cobrands/fixamingata/js.js in order to avoid displaying the installation banner:

// Hide the default PWA installation banner since it covers important UI
// elements (https://github.com/mysociety/fixmystreet/issues/4153).
// deferredPrompt could be used to provide an in-app installation flow.
var deferredPrompt;

addEventListener("beforeinstallprompt", function (event) {
    event.preventDefault();

    deferredPrompt = event;
})

What would be a good place to put this code if we wanted to apply it for all cobrands?

jonkri commented 1 month ago

We've been using the following code in templates/web/fixamingata/footer_extra.html:

-<div class="fms-app-badges">
+<div class="fms-app-badges" style="display: none;">

... and the following code in web/cobrands/fixamingata/js.js:

// Show the app badges if the app is not a PWABuilder progressive web app from
// the iOS App Store.
if (document.cookie.indexOf("app-platform=iOS App Store") === -1) {
    var fmsAppBadges = document.getElementsByClassName("fms-app-badges")[0];

    if (fmsAppBadges) {
        fmsAppBadges.style.display = "block";
    }
}

... in order to hide the icons when running as a PWABuilder app from the iOS App Store.

What would be a good place to put this code if we wanted to apply it for all cobrands?

jonkri commented 1 month ago

Regarding the translation banner, please see #5049.

dracos commented 1 month ago

Will look at translation banner PR.

On the installation banner, I'd ideally want to have a menu item that could appear if that deferredPrompt got set, that could then fire off the installation? On our cobrands we also have a help page such as https://fix.bexley.gov.uk/about/web-app (linked to from the Help page, but that's all) which I quite like.

The app badges - where does the app-platform cookie come from? Is that from the pwabuilder code, it's not in our code? Does that work okay for you? We do set app_platform in the session (so the template could use that variable if it also detected native-ness using this cookie, rather than needing JavaScript, might be a nicer way to go).

jonkri commented 1 month ago

Will look at translation banner PR.

Thanks for merging the PR!

On the installation banner, I'd ideally want to have a menu item that could appear if that deferredPrompt got set, that could then fire off the installation? On our cobrands we also have a help page such as https://fix.bexley.gov.uk/about/web-app (linked to from the Help page, but that's all) which I quite like.

Nice help page!

Regarding firing off the installation. I haven't tested the code but I think something like this should work (assuming there's an installButton element that can be clicked somewhere on the page):

installButton.addEventListener("click", function () {
    deferredPrompt.prompt();

    deferredPrompt.userChoice.then(function (result) {
        if (result.outcome === "accepted") {
            console.log("The installation prompt was accepted.")
        } else if (result.outcome === "dismissed") {
            console.log("The installation prompt was dismissed.")
        }
    });

    installButton.style.display = "none";
    deferredPrompt = null;
});

The app badges - where does the app-platform cookie come from? Is that from the pwabuilder code, it's not in our code? Does that work okay for you? We do set app_platform in the session (so the template could use that variable if it also detected native-ness using this cookie, rather than needing JavaScript, might be a nicer way to go).

It's from the pwabuilder code.

$ grep -Inr app-platform
./src/FixaMinGata/Settings.swift:22:let platformCookie = Cookie(name: "app-platform", value: "iOS App Store")

It has worked okay for us. The app icons are visible outside of the iOS app but hidden inside the iOS app.

Apple rejected the app until we made that change.

jonkri commented 1 month ago

Oh, missed this first time around:

We do set app_platform in the session (so the template could use that variable if it also detected native-ness using this cookie, rather than needing JavaScript, might be a nicer way to go).

It would for sure be nicer to not server side render the icons in the first place if a mobile app is detected (since the images would load faster and the code would be a bit cleaner). However, we're not using the pwa query parameter in our apps. Will the headers be enough for the app to make this determination, you think? (While we could update our apps, we may not be able to get everyone to upgrade to the new version/manifest with the correct start_url value.)

dracos commented 1 month ago

If the native app is setting this cookie, that should also be detectable server side - it would be in $c->req->cookie("app-platform") I think?

jonkri commented 1 month ago

If the native app is setting this cookie, that should also be detectable server side - it would be in $c->req->cookie("app-platform") I think?

Ah, hopefully it would!

I don't think a similar cookie is set on Android but I think we should be able to rely on the android-app:// string there. Is this the correct way to configure a package name?

COBRAND_FEATURES:
  package: 'se.sambruk.fixamingata'
jonkri commented 1 month ago

I wonder what the pwa query parameter is used for.

If we're not using the pwa query parameter, here's a small reason why we may consider removing it:

When I opened up Chrome on an Android device that has the FixaMinGata PWABuilder app and started typing “fixamingata.se” in the address bar, Chrome suggested https://fixamingata.se?pwa as the URL. In other words, Chrome had recognized that it had visited https://fixamingata.se?pwa before (via the PWABuilder app). If I had pressed “Enter” then Chrome would have visited FixaMinGata with the pwa query parameter set, even though the PWABuilder app would not be used. (And Chrome is not switching over to the app if I visit https://fixamingata.se?pwa either.)

So at least in this regards the pwa query parameter doesn't seem to be entirely reliable. It also clutters the URL for the user.

dracos commented 1 month ago

Just on your config point, it would be:

COBRAND_FEATURES:
  android_assetlinks:
    fixamingata:
      package: 'se.sambruk.fixamingata'
jonkri commented 1 month ago

Thanks, @dracos!

I added the fingerprint as well:

COBRAND_FEATURES:
  android_assetlinks:
    fixamingata:
      fingerprints: ['AA:C0:E5:1A:57:45:B8:7E:D4:EB:48:4C:A6:FD:96:FC:B2:BA:7D:93:11:EF:33:CD:E1:2D:6B:EF:0A:BC:7B:A8']
      package: 'se.sambruk.fixamingata'

This will replace (and has the same content as) the static JSON file we served via NGINX until now. 👍