PERF: Check for modal visibility in a more efficient way

PERF: Check for modal visibility in a more efficient way

This code runs on every keyup event in the application, so it needs to be efficient. Previously we were iterating over the whole document using the JQuery :visible selector. Per the JQuery docs at https://api.jquery.com/visible-selector/

Using this selector heavily can have performance implications, as it may force the browser to re-render the page before it can determine visibility. Tracking the visibility of elements via other methods, using a class for example, can provide better performance.

We already had a hidden class on the modal element which we can check, so we can check that instead.

diff --git a/app/assets/javascripts/discourse/app/components/d-modal.js b/app/assets/javascripts/discourse/app/components/d-modal.js
index e338cc6..7a5b3ca 100644
--- a/app/assets/javascripts/discourse/app/components/d-modal.js
+++ b/app/assets/javascripts/discourse/app/components/d-modal.js
@@ -36,7 +36,7 @@ export default Component.extend({
   setUp() {
     $("html").on("keyup.discourse-modal", e => {
       //only respond to events when the modal is visible
-      if ($("#discourse-modal:visible").length > 0) {
+      if (!this.element.classList.contains("hidden")) {
         if (e.which === 27 && this.dismissable) {
           next(() => $(".modal-header button.modal-close").click());
         }

GitHub sha: 76d5e54a

1 Like