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

Add context filtering to API and improve docs #161

Merged
merged 1 commit into from
Jan 27, 2020

Conversation

rikoe
Copy link
Contributor

@rikoe rikoe commented Jan 25, 2020

Addresses #121.

A summary of the type changes I introduced:

  • I introduced a new type for consistent context handling:
type ContextHandler = (context: Context) => void;
  • Changed the channel broadcast listener to be consistent with the rest of the API:
addBroadcastListener(listener: (event: {channel: Channel; context: Context}) => void): Listener;

becomes:

addContextListener(handler: ContextHandler): Listener;

The channel parameter is completely redundant here, as we already now what the channel is from the owning object - if a reference is needed, it can be captured with a closure. This brings the handler in line with the one on the desktop agent, which makes for much easier consumption.

  • Added a new overload for the top-level context listener and channel context listener:
addContextListener(contextType: string, handler: ContextHandler): Listener;
  • Added an optional parameter when getting the current context:
getCurrentContext(contextType?: string): Promise<Context|null>;

Apart from that I have factored out and improved the relevant documentation sections on the website.

- Added overloads for addContextListener that include contextType parameter
- Added optional contextType parameter to getContextValue
- Rename Channel.addBroadcastListener to Channel.addContextListener for consistency
- Change signature of channel context listener to match desktop agent one (remove redundant channel parameter)
- Introduce ContextHandler shared type for context handlers like addContextListener.
- Add documentation for API changes
- Move Channel, Listener, DisplayMetadata and ContextHandler to their own pages
- Cleanup of markdown docs
@rikoe rikoe force-pushed the context-filtering branch from 1300a8b to bab449f Compare January 25, 2020 23:06
@rikoe rikoe added the api FDC3 API Working Group label Jan 25, 2020
* `Error` with a string from the `ChannelError` enumeration.
*/
public broadcast(context: Context): Promise<void>;

/**
* Returns the last context that was broadcast on this channel. All channels initially have no context, until a
* window is added to the channel and then broadcasts. If there is not yet any context on the channel, this method
* will return `null`. The context is also reset back into it's initial context-less state whenever a channel is
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nkolba I have removed the second part - seems like an implementation detail to me. FDC3 has no concept of "windows", so it shouldn't be in there.

@@ -102,34 +103,36 @@ declare class Channel {
* Note that this function can be used without first joining the channel, allowing applications to broadcast on
* channels that they aren't a member of.
*
* This broadcast will be received by all windows that are members of this channel, *except* for the window that
* makes the broadcast. This matches the behaviour of the top-level FDC3 `broadcast` function.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nkolba I have removed this part - seems like an implementation detail to me. FDC3 has no concept of "windows", so it shouldn't be in there.

@nkolba nkolba merged commit 7199d5e into finos:master Jan 27, 2020
@rikoe rikoe deleted the context-filtering branch February 9, 2020 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api FDC3 API Working Group cla-present
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants