search

laravel-notification-channels / onesignal

OneSignal notifications channel for Laravel
MIT License
283 stars 119 forks source link

Add alias for notification channel #85

Closed ahmed-aliraqi closed 5 years ago

ahmed-aliraqi commented 5 years ago

This PR. allows to use onesignal instead of OneSignalChannel::class in via method. Ex : 

/**
 * Get the notification's delivery channels.
 *
 * @param  mixed $notifiable
 * @return array
 */
public function via($notifiable)
{
    return ['onesignal'];
}
LKaemmerling commented 5 years ago

I personally don't like the "alias"es. With the ClassName you have autocompletion from the IDE you use, with the alias of course not.

@Lloople what do you think?

Lloople commented 5 years ago

I’m not very familiar with the framework notifications section 🤔.

I think that if all other notification packages have it it’s something we could offer to the user, but if it’s something new I prefer calling the class as Lukas said.

LKaemmerling commented 5 years ago

After looking into other Notification Channels, they don't provide an alias. So i'll close this for now.