Skip to content

Commit

Permalink
PERF: Check for modal visibility in a more efficient way
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
davidtaylorhq committed Jul 1, 2020
1 parent fc4d748 commit 76d5e54
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion app/assets/javascripts/discourse/app/components/d-modal.js
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand Down

0 comments on commit 76d5e54

Please sign in to comment.