Skip to content

Commit b7de49e

Browse files
committed
Simplify state transitions for speaker notes
Before, we attempted to change state from “popup” to “inline-open” when the speaker note window was closed. We did this by listening to “pagehide” and change the state there. The event fires every time you navigate away from the page, so we had a complex setup where we would reset the state to “popup” when the next page was loaded into the speaker note window. The problem with this is that it’s racy: we could end up in a situation where we set the state to “inline-open” right after the speaker note window was updated. When that happened, we would mark the window as “defunct”, meaning that it was supposed to be closed. With this change, we no longer try to change the state from the speaker note window. If the window is lost (closed), the user will have to click the “Close speaker notes” button in the top-right to reset the state. This should be much more reliable. Long-term, a better solution would be to let the speaker notes fetch the current URL using JavaScript instead of doing it via an actual page navigation. That should allow us to react to “pagehide” events again (since they won’t fire on every page transition).
1 parent 92dafcb commit b7de49e

File tree

2 files changed

+25
-21
lines changed

2 files changed

+25
-21
lines changed

Diff for: speaker-notes.css

+7-6
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,13 @@
1919
width: 40px;
2020
}
2121

22-
.content details summary a {
23-
margin-left: 0.5em;
22+
.content details summary .pop-out {
23+
color: var(--icons);
24+
padding: 0 8px;
25+
cursor: pointer;
26+
transition: color 0.5s;
2427
}
2528

26-
@media only screen and (max-width: 1080px) {
27-
.content details summary a {
28-
display: none;
29-
}
29+
.content details summary .pop-out i:hover {
30+
color: var(--icons-hover);
3031
}

Diff for: speaker-notes.js

+18-15
Original file line numberDiff line numberDiff line change
@@ -78,9 +78,9 @@
7878
popIn.setAttribute("title", "Close speaker notes");
7979
popIn.setAttribute("aria-label", "Close speaker notes");
8080
popIn.classList.add("icon-button");
81-
let i = document.createElement("i");
82-
i.classList.add("fa", "fa-window-close-o");
83-
popIn.append(i);
81+
let popInIcon = document.createElement("i");
82+
popInIcon.classList.add("fa", "fa-window-close-o");
83+
popIn.append(popInIcon);
8484
popIn.addEventListener("click", (event) => {
8585
setState("inline-open");
8686
applyState();
@@ -108,10 +108,20 @@
108108
// Create pop-out button.
109109
let popOutLocation = new URL(window.location.href);
110110
popOutLocation.hash = "#speaker-notes-open";
111-
let popOut = document.createElement("a");
112-
popOut.setAttribute("href", popOutLocation.href);
113-
popOut.setAttribute("target", "speakerNotes");
114-
popOut.classList.add("fa", "fa-external-link");
111+
let popOut = document.createElement("button");
112+
popOut.classList.add("icon-button", "pop-out");
113+
popOut.addEventListener("click", (event) => {
114+
let popup = window.open(popOutLocation.href, "speakerNotes", "popup");
115+
if (popup) {
116+
setState("popup");
117+
applyState();
118+
} else {
119+
window.alert("Could not open popup, please check your popup blocker settings.");
120+
}
121+
})
122+
let popOutIcon = document.createElement("i");
123+
popOutIcon.classList.add("fa", "fa-external-link");
124+
popOut.append(popOutIcon);
115125
summary.append(popOut);
116126
}
117127

@@ -129,11 +139,6 @@
129139

130140
// Create controls for a speaker note window.
131141
function setupSpeakerNotes() {
132-
// Show the notes inline again when the window is closed.
133-
window.addEventListener("pagehide", (event) => {
134-
setState("inline-open");
135-
});
136-
137142
// Hide sidebar and buttons.
138143
document.querySelector("html").classList.remove("sidebar-visible");
139144
document.querySelector("html").classList.add("sidebar-hidden");
@@ -215,9 +220,7 @@
215220
// We encode the kind of page in the location hash:
216221
switch (window.location.hash) {
217222
case "#speaker-notes-open":
218-
// We are on a page in the speaker notes. We need to re-establish the
219-
// popup state so that the main window will hide the notes.
220-
setState("popup");
223+
// We are on a page in the speaker notes.
221224
setupSpeakerNotes();
222225
break;
223226
case "#speaker-notes-defunct":

0 commit comments

Comments
 (0)