Fix bug where speaker notes are not connected to regular window #2675
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Uses a ping-pong based approach via Broadcast channels to detect opens speaker notes.
Also renames the applyState to applyInlinePopupStyle to better caputure the intent of the functionalit. Defunct state is not possible anymore due to the broadcast channel detection mechanism which is self-recovering the correct state. Even if the speaker notes are closed, they are redetected and properly synced when you reopen the closed window manually.
Rename the (set|get)State to (set|get)SpeakerNotesState to better catch the intent.
michael-kerscher
force-pushed
the
fix-speaker-notes-async-bug
branch
from
64db7c2 to
622432a
Compare
djmitche
approved these changes
Feb 28, 2025
There was a problem hiding this comment.
I tested this locally and it seems to work quite well -- thank you!!
I'm not an expert on plain JS. I don't see anything obviously wrong here, and there's basically no risk to landing this, but if you'd like a more skilled review of the implementation feel free to add someone else before merging.
|
lol, or auto-merge :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes bug #2004.
Refactored the communication between the speaker notes window and the regular window by using a Broadcast channel - this is now self-recovering(!) even if speaker notes are closed and manually re-opened!
For better readability and maintainability refactored some string-based states into enum style code and refactored detection of the type of windows (print, speaker note, regular window)
Manually tested the new code and the speaker notes window does not disconnect from the regular window anymore.
This now works way more reliable, even if there are (still) some UI glitches that have been there before already.