-
Notifications
You must be signed in to change notification settings - Fork 39
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
Introduce PresentationRequest and defaultRequest. #146
Conversation
.catch(endSession); | ||
// presId is mandatory when joining a session. | ||
if (presId) { | ||
var request = new PresentationRequest(presUrl); |
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 think in the examples, we are reusing all the definitions from the previous ones, so there's no need to create a new PresentationRequest in each example. It might confuse developers, who will create a separate request availability, starting and joining presentation, etc.
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.
Done.
7ae1062
to
cccab34
Compare
Please take another look. |
@@ -332,8 +333,9 @@ | |||
availability.onchange = function() { handleAvailabilityChange(this.value); }; | |||
}.catch(function() { | |||
// Availability monitoring is not supported by the platform, discovery of presentation | |||
// displays will happen only after startSession() call. Pretend the devices are | |||
// available for simplicity (one could implement the third state for the button). | |||
// displays will happen only after PresentationRequest.start() call. Pretend the |
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.
after request.start()
Thanks for the contribution @mounirlamouri and the comments @avayvod and @mwatson2. I made some inline comments which I can implement in a followup PR. |
Introduce PresentationRequest and defaultRequest.
Editorial changes to pull request #146
Closes #138, #86 and #26.