Skip to content
This repository has been archived by the owner on Sep 7, 2018. It is now read-only.

Add symphony compatibility project #199

Merged
merged 4 commits into from
Jun 13, 2017

Conversation

BenLambertNcl
Copy link
Collaborator

@BenLambertNcl BenLambertNcl commented Jun 13, 2017

Adds a compatibility layer for mapping the symphony API specifications to ContainerJS APIs. most of the APIs are not implemented in ContainerJS yet, but it gives us a simple way of mapping them once they are added.

To use the compatibility layer, add the containerjs-compatibility-api.js file in a script tag in any webpage needing the compatibility. This could be improved by adding within the bundle/preload scripts.

Copy link
Collaborator

@ColinEberhardt ColinEberhardt left a comment

Choose a reason for hiding this comment

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

Looks really good in general - just a few minor changes. Thanks!

return;
}

static registerActivityDetection(throttle: number, callback: Function) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

For these methods I think it would be useful to reference the symphony confluence URL where they are specifed

namespace map {
export class ssf {
static activate(windowName: string) {
// Could use window.focus in containerjs, but no way of selecting window by name yet
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you iterate over all windows to find the one with the given name?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not using ContainerJS. The only method we have is getCurrentWindow, but it should be relatively easy to add this in.

Copy link
Collaborator

Choose a reason for hiding this comment

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

OK, fair enough - that does look like a bit of an omission in our current API - and is something covered in #171

@@ -25,6 +25,7 @@
"dependencies": {
"containerjs-api-bundle": "0.0.2",
"containerjs-api-electron": "0.0.2",
"containerjs-compatibility-api": "0.0.1",
Copy link
Collaborator

Choose a reason for hiding this comment

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

rather than adding to the existing api-demo, I'd prefer to keep this code self-container within containerjs-compatibility-api as I don't think we see these APIs being included in the long-term. Having it all in one place makes it easier to remove ;-)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I only included this as a simple demo for how it works. Do you want to remove the demo, or add a new demo in the containerjs-compatibility-api project?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd prefer to have a new demo in containerjs-compatibility-api - thanks

@BenLambertNcl
Copy link
Collaborator Author

@ColinEberhardt Updated :-)

@BenLambertNcl BenLambertNcl merged commit 0b3ca98 into symphonyoss:master Jun 13, 2017
@BenLambertNcl BenLambertNcl deleted the api-compatible branch June 13, 2017 12:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants