-
Notifications
You must be signed in to change notification settings - Fork 140
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
Raising Intent With AppId but Without InstanceID should open a new instance of the app (DA Reference Implementation) #1556
Conversation
✅ Deploy Preview for fdc3 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
Have reviewed. LGTM.
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'm not yet comfortable with this change and want to test its behaviour (and seek testing by others that have worked on this implementation) before it is merged. The behaviour it is switching to does not match that I've seen in other desktop agent implementations which show a resolver if there are already instances of the appId.
There is no clear specification of the behaviour for this case in the standard - but I believe the convention differs from this. Further the PR description is not clear that the scope of this change is reference implementation (and abstract implementation it is based on) rather than the Standard itself (which could itself be more specific on this point).
Co-authored-by: Kris West <[email protected]>
This tests up fine for me - this is good as we'll be able to achieve without making it possible to explictly request a new instance in FDC3 2.3 |
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.
LGTM
Describe your change
Updates the code in
IntentHandler.ts
and the tests inraise-intent.feature
to cover this edge case.Related Issue
Contributor License Agreement
Review Checklist
DesktopAgent
,Channel
,PrivateChannel
,Listener
,Bridging
)?JSDoc comments on interfaces and types should be matched to the main documentation in /docs
Conformance test definitions should cover all required aspects of an FDC3 Desktop Agent implementation, which are usually marked with a MUST keyword, and optional features (SHOULD or MAY) where the format of those features is defined
The Web Connection protocol and Desktop Agent Communication Protocol schemas must be able to support all necessary aspects of the Desktop Agent API, while Bridging must support those aspects necessary for Desktop Agents to communicate with each other
npm run build
) run and the results checked in?Generated code will be found at
/src/api/BrowserTypes.ts
and/or/src/bridging/BridgingTypes.ts
BaseContext
schema applied viaallOf
(as it is in existing types)?title
anddescription
provided for all properties defined in the schema?npm run build
) run and the results checked in?Generated code will be found at
/src/context/ContextTypes.ts