From 8990410ec8935913c2bea46c90cc48a15b8d9f62 Mon Sep 17 00:00:00 2001 From: Bryce Johnson Date: Tue, 12 Sep 2017 16:16:37 -0400 Subject: Remove emoji-menu from the render tree when hidden. --- app/assets/javascripts/awards_handler.js | 45 +++++++++++++++++++++++++++----- 1 file changed, 39 insertions(+), 6 deletions(-) (limited to 'app/assets/javascripts/awards_handler.js') diff --git a/app/assets/javascripts/awards_handler.js b/app/assets/javascripts/awards_handler.js index 22fa1f2a609..5313d834b40 100644 --- a/app/assets/javascripts/awards_handler.js +++ b/app/assets/javascripts/awards_handler.js @@ -23,6 +23,9 @@ const categoryLabelMap = { flags: 'Flags', }; +const IS_VISIBLE = 'is-visible'; +const IS_RENDERED = 'is-rendered'; + class AwardsHandler { constructor(emoji) { this.emoji = emoji; @@ -50,7 +53,7 @@ class AwardsHandler { if (!$target.closest('.emoji-menu').length) { if ($('.emoji-menu').is(':visible')) { $('.js-add-award.is-active').removeClass('is-active'); - $('.emoji-menu').removeClass('is-visible'); + this.hideMenuElement($('.emoji-menu')); } } }); @@ -87,12 +90,12 @@ class AwardsHandler { if ($menu.length) { if ($menu.is('.is-visible')) { $addBtn.removeClass('is-active'); - $menu.removeClass('is-visible'); + this.hideMenuElement($menu); $('.js-emoji-menu-search').blur(); } else { $addBtn.addClass('is-active'); this.positionMenu($menu, $addBtn); - $menu.addClass('is-visible'); + this.showMenuElement($menu); $('.js-emoji-menu-search').focus(); } } else { @@ -102,7 +105,7 @@ class AwardsHandler { $addBtn.removeClass('is-loading'); this.positionMenu($createdMenu, $addBtn); return setTimeout(() => { - $createdMenu.addClass('is-visible'); + this.showMenuElement($createdMenu); $('.js-emoji-menu-search').focus(); }, 200); }); @@ -240,7 +243,8 @@ class AwardsHandler { if (gl.utils.isInIssuePage() && !isMainAwardsBlock) { const id = votesBlock.attr('id').replace('note_', ''); - $('.emoji-menu').removeClass('is-visible'); + this.hideMenuElement($('.emoji-menu')); + $('.js-add-award.is-active').removeClass('is-active'); const toggleAwardEvent = new CustomEvent('toggleAward', { detail: { @@ -260,7 +264,8 @@ class AwardsHandler { return typeof callback === 'function' ? callback() : undefined; }); - $('.emoji-menu').removeClass('is-visible'); + this.hideMenuElement($('.emoji-menu')); + return $('.js-add-award.is-active').removeClass('is-active'); } @@ -528,6 +533,34 @@ class AwardsHandler { return $matchingElements.closest('li').clone(); } + /* showMenuElement and hideMenuElement are performance optimizations. We use + * opacity to show/hide the emoji menu, because we can animate it. But opacity + * leaves hidden elements in the render tree, which is unacceptable given the number + * of emoji elements in the emoji menu (5k+). To get the best of both worlds, we separately + * apply IS_RENDERED to add/remove the menu from the render tree and IS_VISIBLE to animate + * the menu being opened and closed. */ + + showMenuElement($emojiMenu) { + // enqueues animation as a microtask, so it begins ASAP once IS_RENDERED added + Promise.resolve().then(() => { // eslint-disable-line promise/catch-or-return + $emojiMenu.addClass(IS_VISIBLE); + }); + + $emojiMenu.addClass(IS_RENDERED); + } + + hideMenuElement($emojiMenu) { + $emojiMenu.on(transitionEndEventString, (e) => { + if (e.currentTarget === e.target) { + $emojiMenu + .removeClass(IS_RENDERED) + .off(transitionEndEventString); + } + }); + + $emojiMenu.removeClass(IS_VISIBLE); + } + destroy() { this.eventListeners.forEach((entry) => { entry.element.off.call(entry.element, ...entry.args); -- cgit v1.2.3 From 8349782f7f9774f72de37da75e9956fca8e3d5cb Mon Sep 17 00:00:00 2001 From: Bryce Johnson Date: Wed, 20 Sep 2017 15:11:36 -0400 Subject: Reverse order in showMenuElement for readability. --- app/assets/javascripts/awards_handler.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'app/assets/javascripts/awards_handler.js') diff --git a/app/assets/javascripts/awards_handler.js b/app/assets/javascripts/awards_handler.js index 5313d834b40..2fcd9d3d4a2 100644 --- a/app/assets/javascripts/awards_handler.js +++ b/app/assets/javascripts/awards_handler.js @@ -541,12 +541,12 @@ class AwardsHandler { * the menu being opened and closed. */ showMenuElement($emojiMenu) { + $emojiMenu.addClass(IS_RENDERED); + // enqueues animation as a microtask, so it begins ASAP once IS_RENDERED added Promise.resolve().then(() => { // eslint-disable-line promise/catch-or-return $emojiMenu.addClass(IS_VISIBLE); }); - - $emojiMenu.addClass(IS_RENDERED); } hideMenuElement($emojiMenu) { -- cgit v1.2.3 From 014bbe6562d7feb374f7c1eaa14846bb56f7c2a6 Mon Sep 17 00:00:00 2001 From: Bryce Johnson Date: Wed, 20 Sep 2017 15:17:08 -0400 Subject: Remove eslint-disable, and use arrow function. --- app/assets/javascripts/awards_handler.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'app/assets/javascripts/awards_handler.js') diff --git a/app/assets/javascripts/awards_handler.js b/app/assets/javascripts/awards_handler.js index 2fcd9d3d4a2..5f9695cc443 100644 --- a/app/assets/javascripts/awards_handler.js +++ b/app/assets/javascripts/awards_handler.js @@ -544,9 +544,8 @@ class AwardsHandler { $emojiMenu.addClass(IS_RENDERED); // enqueues animation as a microtask, so it begins ASAP once IS_RENDERED added - Promise.resolve().then(() => { // eslint-disable-line promise/catch-or-return - $emojiMenu.addClass(IS_VISIBLE); - }); + return Promise.resolve() + .then(() => $emojiMenu.addClass(IS_VISIBLE)); } hideMenuElement($emojiMenu) { -- cgit v1.2.3