Repository navigation
Fix example 5.5 and move to ECMAScript2015 - #358
Conversation
| var disconnectBtn = document.getElementById("disconnectBtn"); | ||
| var stopBtn = document.getElementById("stopBtn"); | ||
| stopBtn.onclick = function () { connection && connection.terminate(); }; | ||
| let stopBtn = document.querySelector("#stopBtn"); |
There was a problem hiding this comment.
Can you use const instead of let for the *Btn variables?
| connection.onterminate(); | ||
| connection.onterminate = undefined; | ||
|
|
||
| connection.terminate(); |
There was a problem hiding this comment.
This could terminate the presentation when it's reconnected to, per example 5.3, if the old connection hasn't fired onclose yet.
If we want to guarantee that at most one connection to the presentation is open, then maybe this could be:
if (connection && connection != newConnection && connection.state != 'closed') {
connection.onclosed = undefined;
connection.close();
}
| let disconnectBtn = document.querySelector("#disconnectBtn"); | ||
|
|
||
| stopBtn.onclick = _ => { | ||
| connection && connection.terminate(); |
There was a problem hiding this comment.
I think you'll need && here and below.
There was a problem hiding this comment.
Both seems to work, but let me make the change
markafoltz
left a comment
There was a problem hiding this comment.
LGTM with one more fix. Thanks for the PR!
|
|
||
| function setConnection(newConnection) { | ||
| // Disconnect from existing presentation, if not attempting to reconnect | ||
| if (connection && connection != newConnection && connection.state != 'closed') { |
There was a problem hiding this comment.
&& should be &&
|
To move things forward I'll merge this and submit a follow-up pull request to make any necessary further changes. |
Fix errors in example 5.5 like missing reconnectBtn element and move to newer ECMAScript features where it makes sense.