mdToast: API improvement for $mdToast.show(...)

[ahmedshuhel]

For a simple toasts that shows a simple text message. It would be nice to have an api like:

$mdToast.show('Hello!');

rather,

$mdToast.show($mdToast.simple().content('Hello!'));

What do you think? I can help with that.

[ThomasBurleson]

@rschmukler - This seems like a reasonable API improvement to show( options ). If options is:

• String - then show() internally performs as show($mdToast.simple().content('Hello!'));
• Hashmap - then perform as currently implemented.

[rschmukler]

@ThomasBurleson and @ahmedshuhel, I am hesitant to overload the API for an $interimElementService.show to support a string as it makes $mdToast different from any other service. Instead, we've implemented the following to work:

$mdToast.show($mdToast.simple('Hello!'));

[ahmedshuhel]

@rschmukler and @ThomasBurleson,
I would have gone with @ThomasBurleson's way. However, $mdToast.simple(...) is much better than the previous. But in in ideal world, I would like to use, $mdToast.show('simple message') 😄

$mdToast is already different from any other service from the consumer point of view. So, even as developers we should treat it differently.

[gkalpak]

I am not particularly keen on the $mdToast.show($mdToast.simple('Hello!'));; it still seems too verbose.
Why not a new method on $mdToast (no overloading). Is there any reason we wouldn't want the interimElement to have different (extra) methods ?

[ThomasBurleson]

@rschmukler 👍

[ilanbiala]

@rschmukler a "simple" toast should just be Toast.show('Success!');...that's simple!

[rschmukler]

@ilanbiala the advantage of not doing that is that it keeps Toast consistent with other $mdInterimElementService based services (eg. Dialog). Overloading it requires either a wrapper function (which calls the actual $mdToast.show) or adding in overloading at the $mdInterimElementService level, neither of which feels clean.

To @gkalpak's point, I'd be more inclined to provide that style of handling this. Ie. for methods which we define constructors ($mdToast.simple, $mdDialog.alert, etc) we also provide a method prefixed w/ show which handles the verbosity for you (ie. $mdToast.showSimple and $mdDialog.showAlert). Let me know what you guys think? (@ilanbiala, @gkalpak, @ThomasBurleson , @ajoslin and anyone else who might have an opinion)

Also, keep in mind that you can store those results, in a variable, which could make your code cleaner, if you are using the same toast in multiple places. Eg.

module.controller('MyCtrl', function($mdToast, MyService, MyOtherService) {
var self = this;
var loadFailed = $mdToast.simple('Failed to load resources');

MyService.fetch().then(function(resources) {
  self.resources = resources;
}, function() {
  $mdToast.show(loadFailed);
});

MyOtherService.fetch().then(function(otherResources) {
  self.otherResources = otherResources;
}, function() {
  $mdToast.show(loadFailed);
});

[ilanbiala]

@rschmukler it really isn't intuitive for me to have a Toast notification in a variable. Same for $mdDialog really, but I guess that's something others should weigh in on, too. I would think the API should be something like this (assuming wrapped in a controller with the correct injections):

$mdToast.simple('Success');

$mdToast
  .show({
    content: 'Success',
    position: 'bottom right',
    hideDelay: 5000
  });

$mdDialog.simple('Create user', 'This is a sample create user dialog', 'Create');

$mdDialog
  .show({
    type: 'confirm',
    title: 'Create user',
    content: 'This is how to create a user',
    ok: 'Create'
  });

Maybe the $mdDialog.simple() is too simple because it isn't clear what each parameter does, but I think many people appreciate less characters and a one-liner that reads pretty easily after a few occurrences compared to $mdDialog.simple().title().content().ok().show(); (Yes, I know .show() isn't a valid method, but I think you get the point).

[gkalpak]

@ilanbiala:

it really isn't intuitive for me to have a Toast notification in a variable

You are not keeping the notification in the variable, but rather the configuration (preset) for creating the notification, which makes perfect sense imo.
That said, an object instead of a function call chain for creating that configuration does indeed make more sense.

@rschmukler: I agree that overloading the interimElement methods is probably not a good idea, but overloading specific interim elements (such as mdToast, mdDialog etc) should be the way to go.
So, I am all for showXyz() methods 👍