rust/comprehensive-rust
1
0
Fork 0
mirror of https://github.com/google/comprehensive-rust.git synced 2025-04-17 22:16:18 +02:00
Code Issues Releases Activity

Simplify state transitions for speaker notes

Browse Source 
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).
This commit is contained in:
Martin Geisler 2023-01-12 12:26:07 +01:00
parent 92dafcb676
commit b7de49e1b8
2 changed files with 25 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
speaker-notes.css
View File

@ -19,12 +19,13 @@
width: 40px;
}
.content details summary a {
margin-left: 0.5em;
.content details summary .pop-out {
color: var(--icons);
padding: 0 8px;
cursor: pointer;
transition: color 0.5s;
}
@media only screen and (max-width: 1080px) {
.content details summary a {
display: none;
}
.content details summary .pop-out i:hover {
color: var(--icons-hover);
}

33
speaker-notes.js
View File

@ -78,9 +78,9 @@
popIn.setAttribute("title", "Close speaker notes");
popIn.setAttribute("aria-label", "Close speaker notes");
popIn.classList.add("icon-button");
let i = document.createElement("i");
i.classList.add("fa", "fa-window-close-o");
popIn.append(i);
let popInIcon = document.createElement("i");
popInIcon.classList.add("fa", "fa-window-close-o");
popIn.append(popInIcon);
popIn.addEventListener("click", (event) => {
setState("inline-open");
applyState();
@ -108,10 +108,20 @@
// Create pop-out button.
let popOutLocation = new URL(window.location.href);
popOutLocation.hash = "#speaker-notes-open";
let popOut = document.createElement("a");
popOut.setAttribute("href", popOutLocation.href);
popOut.setAttribute("target", "speakerNotes");
popOut.classList.add("fa", "fa-external-link");
let popOut = document.createElement("button");
popOut.classList.add("icon-button", "pop-out");
popOut.addEventListener("click", (event) => {
let popup = window.open(popOutLocation.href, "speakerNotes", "popup");
if (popup) {
setState("popup");
applyState();
} else {
window.alert("Could not open popup, please check your popup blocker settings.");
}
})
let popOutIcon = document.createElement("i");
popOutIcon.classList.add("fa", "fa-external-link");
popOut.append(popOutIcon);
summary.append(popOut);
}
@ -129,11 +139,6 @@
// Create controls for a speaker note window.
function setupSpeakerNotes() {
// Show the notes inline again when the window is closed.
window.addEventListener("pagehide", (event) => {
setState("inline-open");
});
// Hide sidebar and buttons.
document.querySelector("html").classList.remove("sidebar-visible");
document.querySelector("html").classList.add("sidebar-hidden");
@ -215,9 +220,7 @@
// We encode the kind of page in the location hash:
switch (window.location.hash) {
case "#speaker-notes-open":
// We are on a page in the speaker notes. We need to re-establish the
// popup state so that the main window will hide the notes.
setState("popup");
// We are on a page in the speaker notes.
setupSpeakerNotes();
break;
case "#speaker-notes-defunct":