From 2bda335f1cdbb8bc15c459904cc701c728c8bf99 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Fri, 21 Oct 2022 17:27:10 -0500 Subject: [PATCH] Clean up spurious logs when SSR rendering Hydrogen (#106) **Before:** ``` Child printed something to stdout: Mounting Hydrogen... Child printed something to stderr: Skipping `addedHomservers` read from LocalStorage since LocalStorage is not available Child printed something to stdout: Completed mounting Hydrogen: 22.188ms Child printed something to stdout: 2 uncaughtException TypeError: this.dialogNode.close is not a function at ModalView.closeModal (C:\Users\MLM\Documents\GitHub\element\matrix-public-archive\shared\views\ModalView.js:115:21) at Timeout._onTimeout (C:\Users\MLM\Documents\GitHub\element\matrix-public-archive\shared\views\ModalView.js:87:18) at listOnTimeout (node:internal/timers:559:17) at processTimers (node:internal/timers:502:7) ``` **After:** ``` Child printed something to stdout: Mounting Hydrogen... Child printed something to stderr: Skipping `addedHomservers` read from LocalStorage since LocalStorage is not available Child printed something to stdout: Completed mounting Hydrogen: 14.2ms ``` --- shared/lib/archive-history.js | 8 ++++---- shared/views/ModalView.js | 11 +++++++++-- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/shared/lib/archive-history.js b/shared/lib/archive-history.js index 29bee7e..1f619c1 100644 --- a/shared/lib/archive-history.js +++ b/shared/lib/archive-history.js @@ -28,20 +28,20 @@ class ArchiveHistory extends History { // downstream call of `urlRouter.attach()` which we do when bootstraping // everything. if (window.history) { - let replacingUrl = window.location.search + url; + let replacingUrl = document?.location?.search + url; // This is a way to make sure the hash gets cleared out if (url === '') { - replacingUrl = window.location.pathname + window.location.search; + replacingUrl = document?.location?.pathname + document?.location?.search; } super.replaceUrlSilently(replacingUrl); } } pushUrlSilently(url) { - let replacingUrl = window.location.search + url; + let replacingUrl = document?.location?.search + url; // This is a way to make sure the hash gets cleared out if (url === '') { - replacingUrl = window.location.pathname + window.location.search; + replacingUrl = document?.location?.pathname + document?.location?.search; } super.pushUrlSilently(replacingUrl); } diff --git a/shared/views/ModalView.js b/shared/views/ModalView.js index ba133c5..585e635 100644 --- a/shared/views/ModalView.js +++ b/shared/views/ModalView.js @@ -81,9 +81,12 @@ class ModalView extends TemplateView { // The dialog has to be in the DOM before we can call `showModal`, etc. // Assume this view will be mounted in the parent DOM straight away. requestAnimationFrame(() => { - if (open) { + // Prevent doing extra work if the modal is already closed or open and already + // matches our intention + const isAlreadyOpen = !!dialog.getAttribute('open'); + if (open && !isAlreadyOpen) { this.showModal(); - } else { + } else if (isAlreadyOpen) { this.closeModal(); } }); @@ -98,6 +101,10 @@ class ModalView extends TemplateView { } onDialogClicked(event) { + // Close the dialog when the backdrop is clicked. The `::backdrop` is considered + // part of the `dialogNode` but we have a `modalInner` element that stops clicks on + // the dialog content itself counting as a click on it. So the only clicks to the + // dialog will be on the backdrop and we can safely assume they meant to close it. if (event.target === this.dialogNode) { this.closeModal(); }