1215026 - Create JSM snackbar API

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Sebastian Kaspari (:sebastian; :pocmo)]

In bug 1157526 we want to replace various toasts with snackbars. Often we rely on NativeWindow.toast to display the toasts, for example when closing a tab (bug 1204760). We should create a NativeWindow.snackbar API. This allows us as well as add-ons to use snackbars. The toast and snackbar APIs are very similar.

NativeWindow.toast:
https://developer.mozilla.org/en-US/Add-ons/Firefox_for_Android/API/NativeWindow/toast

Android Snackbar API:
https://developer.android.com/reference/android/support/design/widget/Snackbar.html

Comment 1

[Sebastian Kaspari (:sebastian; :pocmo)]

I was thinking about creating a follow-up bug to deprecate NativeWindow.toast afterwards. But seeing that Android did not deprecate toasts in favor of snackbars, I don't think we should push into this direction.

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

Let's not add anything to NativeWindow. I wish I had never created NativeWindow. Let's just create a new JSM.

I was going to also suggest that we just change the implementation of Toast to show snackbars instead of toasts, but then Sebastian mentioned that Android still allows both toasts and snackbars to be used, so maybe we should *add* Snackbar and not *replace* Toast. I would hope that the APIs would be similar though.

Comment 3

[Sebastian Kaspari (:sebastian; :pocmo)]

(In reply to Mark Finkle (:mfinkle) from comment #2)
> Let's not add anything to NativeWindow. I wish I had never created
> NativeWindow. Let's just create a new JSM.

Agreed! I just looked at how toasts are shown from JS. I'm okay with implementing whatever is the best solution nowadays.

Comment 4

[Nick Alexander :nalexander [he/him]]

(In reply to Mark Finkle (:mfinkle) from comment #2)
> Let's not add anything to NativeWindow. I wish I had never created
> NativeWindow. Let's just create a new JSM.

+1 to this.

> I was going to also suggest that we just change the implementation of Toast
> to show snackbars instead of toasts, but then Sebastian mentioned that
> Android still allows both toasts and snackbars to be used, so maybe we
> should *add* Snackbar and not *replace* Toast. I would hope that the APIs
> would be similar though.

Not replacing the existing API leaves a potentially difficult deprecation to the future.  I prefer replacing the existing API, although I could be convinced otherwise.

Comment 5

[Sebastian Kaspari (:sebastian; :pocmo)]

(In reply to Nick Alexander :nalexander from comment #4)
> Not replacing the existing API leaves a potentially difficult deprecation to
> the future.  I prefer replacing the existing API, although I could be
> convinced otherwise.

There's a risk of the Snackbar changing/vanishing too. Toasts are a system feature since Android 1.0. Snackbars are only in the Android Design Support Library right now.

The Material design guidelines mention both Toasts and Snackbars - without preferring one over the other. However it then continues with just describing guidelines for Snackbars:
https://www.google.de/design/spec/components/snackbars-toasts.html

Comment 6

[Sebastian Kaspari (:sebastian; :pocmo)]

I created bug 1216051 for deciding what to do with NativeWindow.toast.

Comment 7

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: 1215026-snackbar-api.patch

I've build this by looking at the toast implementation and other JSMs. :)

Comment 8

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 8676401 [details] [diff] [review]
1215026-snackbar-api.patch


>diff --git a/mobile/android/modules/Snackbars.jsm b/mobile/android/modules/Snackbars.jsm

>+var Snackbars = {

>+  _callbacks: {},
>+
>+  init: function() {
>+    Services.obs.addObserver(this, "Snackbar:Click", false)
>+  },
>+
>+  show: function(aMessage, aDuration, aOptions) {
>+    let msg = {
>+      type: 'Snackbar:Show',
>+      message: aMessage,
>+      duration: aDuration,
>+    };
>+
>+    if (aOptions && aOptions.action) {
>+      msg.action = {
>+        id: uuidgen.generateUUID().toString()
>+      };
>+
>+      if (aOptions.action.label) {
>+        msg.action.label = aOptions.action.label;
>+      }
>+
>+      this._callbacks[msg.action.id] = aOptions.action.callback;
>+    }
>+
>+    Services.androidBridge.handleGeckoMessage(msg);
>+  },
>+
>+  observe: function(aSubject, aTopic, aData) {
>+    if (this._callbacks[aData]) {
>+      this._callbacks[aData]();
>+      delete this._callbacks[aData];
>+    }
>+  }
>+};
>+
>+Snackbars.init();

Take a look at the Toasts.jsm part of the patch here:
https://bug1052276.bmoattachments.org/attachment.cgi?id=8471763

We should not need to hold callbacks and do an init() if we use Messaging.jsm

Also, I have an idea for a really simple test, modeled after testTrackingProtection.java / .js where we only test for the "Snackbar:Show" message and data, and not try to verify the Snackbar UI was actually shown.

Comment 9

[:Margaret Leibovic]

(In reply to Mark Finkle (:mfinkle) from comment #8)

> Also, I have an idea for a really simple test, modeled after
> testTrackingProtection.java / .js where we only test for the "Snackbar:Show"
> message and data, and not try to verify the Snackbar UI was actually shown.

+1 let's add a test!

Comment 10

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: 1215026-snackbar-api-v2.patch

Thanks! So much nicer with EventCallback.

This patch also adds a simple test. I had to refactor the callback handling a bit because Snackbar does not guarantee to only call the action callback xor the dismiss callback.

Comment 11

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 8676800 [details] [diff] [review]
1215026-snackbar-api-v2.patch

LGTM!

Comment 12

[Sebastian Kaspari (:sebastian; :pocmo)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=87db0ab3a9b5

Comment 13

[Sebastian Kaspari (:sebastian; :pocmo)]

https://hg.mozilla.org/integration/fx-team/rev/dc5816aa157cfd4414a9bd30358621eb95e0c418
Bug 1215026 - Create API for Snackbars. r=mfinkle

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/dc5816aa157c