-
Notifications
You must be signed in to change notification settings - Fork 239
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
Widget API: initial skeleton #2420
Changes from 7 commits
a5df9c5
8bed3a7
1247f0c
090f425
d3fe2f8
d25c800
73ee5bf
012a215
463ea60
7f90b28
583ce4f
448435d
7793bcf
02cde35
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
//! Client widget API implementation. | ||
|
||
use crate::room::Room as JoinedRoom; | ||
|
||
mod permissions; | ||
mod widget; | ||
|
||
pub use self::{ | ||
permissions::{Permissions, PermissionsProvider}, | ||
widget::{Comm, Info, Widget}, | ||
}; | ||
|
||
/// Starts a client widget API state machine for a given `widget` in a given joined `room`. | ||
/// The function returns once the widget is disconnected or any terminal error occurs. | ||
daniel-abramov marked this conversation as resolved.
Show resolved
Hide resolved
|
||
/// | ||
/// Not implemented yet, currently always panics. | ||
pub async fn run_widget_api( | ||
_room: JoinedRoom, | ||
_widget: widget::Widget, | ||
_permissions_provider: impl permissions::PermissionsProvider, | ||
) -> Result<(), ()> { | ||
todo!() | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
//! Types and traits related to the permissions that a widget can request from a client. | ||
|
||
use async_trait::async_trait; | ||
|
||
/// Must be implemented by a component that provides functionality of deciding whether | ||
/// a widget is allowed to use certain capabilities (typically by providing a prompt to the user). | ||
#[async_trait] | ||
pub trait PermissionsProvider: Send + Sync + 'static { | ||
/// Receives a request for given permissions and returns the actual permissions that | ||
/// the clients grants to a given widget (usually by prompting the user). | ||
async fn acquire_permissions(&self, permissions: Permissions) -> Permissions; | ||
} | ||
|
||
/// Permissions that a widget can request from a client. | ||
#[derive(Debug)] | ||
pub struct Permissions { | ||
/// Types of the messages that a widget wants to be able to fetch. | ||
pub read: Vec<MessageType>, | ||
/// Types of the messages that a widget wants to be able to send. | ||
pub send: Vec<MessageType>, | ||
} | ||
|
||
/// An alias for an actual type of the messages that is going to be used with a permission request. | ||
pub type MessageType = String; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is for a Matrix event type, and widgets might interact with both message events and state events, right? If yes, this type alias doesn't need to exist, we can use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right (if I remember correctly). I actually wanted to also use stronger types here and had a discussion with @toger5, but as far as I can remember, @toger5's suggestion was that it can be any event type in theory and that there was no simple way to map them (no enum with a function There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are other "event" types that aren't part of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Widgets are allowed to introduce new unspecced message types. Just arbitrary strings for their own usecases. Does TimelineEevntType have a "other" enum case. If so this would very much be a better solution. If not it would be a shame if a widget is working with its own timeline event types but the widget driver is giving a parsing error. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yes,any string can be converted There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice! Updated. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
//! The description of a widget. | ||
|
||
use tokio::sync::mpsc::{Receiver, Sender}; | ||
|
||
/// Describes a widget. | ||
#[derive(Debug)] | ||
pub struct Widget { | ||
/// Information about the widget. | ||
pub info: Info, | ||
/// Communication channels with a widget. | ||
pub comm: Comm, | ||
} | ||
|
||
/// Information about a widget. | ||
#[derive(Debug)] | ||
pub struct Info { | ||
/// Widget's unique identifier. | ||
pub id: String, | ||
/// Whether or not the widget should be initialized on load message (`ContentLoad` message), | ||
/// or upon creation/attaching of the widget to the SDK's state machine that drives the API. | ||
pub init_on_load: bool, | ||
daniel-abramov marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
/// Communication "pipes" with a widget. | ||
#[derive(Debug)] | ||
pub struct Comm { | ||
/// Raw incoming messages from the widget (normally, formatted as JSON). | ||
pub from: Receiver<String>, | ||
daniel-abramov marked this conversation as resolved.
Show resolved
Hide resolved
|
||
/// Raw outgoing messages from the client (SDK) to the widget (normally formatted as JSON). | ||
pub to: Sender<String>, | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would rename this module
widget
instead ofwidget_api
. Just a personal taste compared to the rest of the SDK.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Though in that case, we should not have a
widget
sub-module. Which is fine by me really, the items from that could be part of this module just as well.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also prefer short and concise names, I think the only reason we chose
widget_api
originally was that we thought that if we call itwidget
, it might be confused with the widget implementation (not the API, in our case the client widget API), but I'm totally fine with renaming it towidget
if that's your preference folks!There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The client widget API couldn't live in this crate though, right? In that case, let's do the rename as suggested by Ivan.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated.
The actual implementation of the
run()
function and its internals. Originally I thought that it would live there, but maybe you're right and it's better to place the client widget API in a separate module or something like this.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, then I guess I misunderstood what you meant by "client widget API". Anyways, let's figure this out when it comes to the next PR.