Sebastian Kaspari (:sebastian; :pocmo) Assignee
	Description • 9 years ago

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

	Sebastian Kaspari (:sebastian; :pocmo) Assignee
	Comment 1 • 9 years ago

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.

	Mark Finkle (:mfinkle) (use needinfo?)
	Comment 2 • 9 years ago

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.

	Sebastian Kaspari (:sebastian; :pocmo) Assignee
	Comment 3 • 9 years ago

(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.

	Nick Alexander :nalexander [he/him]
	Comment 4 • 9 years ago

(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.

	Sebastian Kaspari (:sebastian; :pocmo) Assignee
	Comment 5 • 9 years ago

(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

	Sebastian Kaspari (:sebastian; :pocmo) Assignee
	Comment 6 • 9 years ago

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

	Sebastian Kaspari (:sebastian; :pocmo) Assignee
	Comment 7 • 9 years ago

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

	Mark Finkle (:mfinkle) (use needinfo?)
	Comment 8 • 9 years ago

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.

	:Margaret Leibovic
	Comment 9 • 9 years ago

(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!

	Sebastian Kaspari (:sebastian; :pocmo) Assignee
	Comment 10 • 9 years ago

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.

	Mark Finkle (:mfinkle) (use needinfo?)
	Comment 11 • 9 years ago

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

LGTM!

	Sebastian Kaspari (:sebastian; :pocmo) Assignee
	Comment 12 • 9 years ago

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

	Sebastian Kaspari (:sebastian; :pocmo) Assignee
	Comment 13 • 9 years ago

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

	Carsten Book [:Tomcat]
	Comment 14 • 9 years ago

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