Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: notifications crate #61

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from
Draft

Conversation

Aloso
Copy link

@Aloso Aloso commented Mar 30, 2019

Displaying notifications on the web.

This is a work in progress and hasn't been tested. Feedback is very welcome.

This API comes in two flavors: A callback style and Futures API.

Before a notification can be displayed, the user of the browser has to give his permission.

1. Callback style

use gloo_notifications::Notification;

Notification::request_permission_map(|mut builder| {
    builder.title("Notification title").show();
});

Notification::request_permission_map_or(|mut builder| {
    builder.title("Notification title")
        .body("Notification body")
        .show();
}, |_| {
    // in case the permission is denied
});

// short form, if you only need a title
Notification::request_permission_with_title("Notification title");

2. Future API:

use gloo_notifications::Notification;

Notification::request_permission()
    .map(|mut builder| {
        builder.title("Notification title").show();
    })
    .map_err(|_| {
        // in case the permission is denied
    })

Adding event listeners

I have only implemented onclick so far, since I don't know how event listeners should be implemented.

use gloo_notifications::Notification;

Notification::request_permission_map(|mut builder| {
    let notification = builder
        .title("Notification title")
        .show();
    notification.onclick(|_| { ... });
})

Macro

use gloo_notifications::{Notification, notification};

// requests permission, then displays the notification
// and adds a "click" event listener
notification! {
    title: "Hello World",
    body: "Foo",
    icon: "/assets/notification.png";
    onclick: |_| {}
}

@DesmondWillowbrook
Copy link

Looking at the usage, it looks like this API is not mirroring the JS one and rather adding a lot of its own API?

@hamza1311
Copy link
Collaborator

Looking at the usage, it looks like this API is not mirroring the JS one and rather adding a lot of its own API?

A mirrored API already exists (see web_sys). gloo is supposed to provide an API that is idiomatic Rust, not an API that that mirrors the JS one

@DesmondWillowbrook
Copy link

I agree that the API is intended to be Rust-like, but my impression of the current aim (for the low-level libraries in Gloo) was to remove any bindgen stuff and use only Rust native types in the APIs (and remove extraneous Results from web_sys, wrap JS errors in proper error enums, etc.), then advance to middle-to-high level APIs which are "opinionated", i.e. make stuff ergonomic.

Specifically, looking at the builder pattern which is implemented here or the notification macro makes it feel like a higher level API.

These additions certainly look helpful, just that I don't think that the first implementation for notifications into gloo should be even that opinionated. Maybe once the JS API surface has been Rustified, then we can look at building it further.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants