From e87f5af465d87ae0ada07fcb55e0d2fd96dd746c Mon Sep 17 00:00:00 2001 From: Denis Date: Thu, 30 Mar 2023 13:25:55 +0200 Subject: [PATCH 1/4] Apply suggestions from code review JS cleanup after review Co-authored-by: Molkobain --- js/layouts/tab-container/regular-tabs.js | 9 +++++---- js/layouts/tab-container/scrollable-tabs.js | 5 +++-- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/js/layouts/tab-container/regular-tabs.js b/js/layouts/tab-container/regular-tabs.js index fc5cc5f7c8..24b92cd12f 100644 --- a/js/layouts/tab-container/regular-tabs.js +++ b/js/layouts/tab-container/regular-tabs.js @@ -126,13 +126,14 @@ $.widget( "itop.regulartabs", $.ui.tabs, { }, // JQuery UI overload disable: function(index){ - let panel = this._getPanelForTab( index ); - panel.css({display: 'none'}); + const panel = this._getPanelForTab( index ); + panel.hide(); this._super( index ); }, + // JQuery UI overload enable: function(index) { - let panel = this._getPanelForTab( index ); - panel.css({display: 'block'}); + const panel = this._getPanelForTab( index ); + panel.show(); this._super( index ); }, }); diff --git a/js/layouts/tab-container/scrollable-tabs.js b/js/layouts/tab-container/scrollable-tabs.js index c529897e23..91b679e2c5 100644 --- a/js/layouts/tab-container/scrollable-tabs.js +++ b/js/layouts/tab-container/scrollable-tabs.js @@ -379,12 +379,13 @@ $.widget( "itop.scrollabletabs", $.ui.tabs, { }, // JQuery UI overload disable: function(index){ - let panel = this._getPanelForTab( this.tabs[index] ); + const panel = this._getPanelForTab( this.tabs[index] ); panel.css({display: 'none'}); this._super( index ); }, + // JQuery UI overload enable: function(index) { - let panel = this._getPanelForTab( this.tabs[index] ); + const panel = this._getPanelForTab( this.tabs[index] ); panel.css({display: 'block'}); this._super( index ); }, From b9a00b15f5f83599e70d3e951a93fb4b3b2cb0f6 Mon Sep 17 00:00:00 2001 From: "denis.flaven@combodo.com" Date: Thu, 30 Mar 2023 14:16:26 +0200 Subject: [PATCH 2/4] Disable tabs by ID instead of index Disabled tabs are visible (with a 'not-allowed' cursor) instead of being hidden from the extra tabs menu. --- css/backoffice/utils/helpers/_misc.scss | 4 ++ js/layouts/tab-container/tab-container.js | 53 +++++++++++++++-------- 2 files changed, 39 insertions(+), 18 deletions(-) diff --git a/css/backoffice/utils/helpers/_misc.scss b/css/backoffice/utils/helpers/_misc.scss index 845208ba1e..93ee5dc8ed 100644 --- a/css/backoffice/utils/helpers/_misc.scss +++ b/css/backoffice/utils/helpers/_misc.scss @@ -35,6 +35,10 @@ $ibo-sticky-sentinel-bottom--height: $ibo-sticky-sentinel--height !default; opacity: 1 !important; /* Note: !important is necessary as it needs to overload any standard rules */ } +.ibo-is-disabled { + cursor: not-allowed !important; /* Note: !important is necessary as it needs to overload any standard rules */ +} + /****************************/ /* Disposition / alignement */ /****************************/ diff --git a/js/layouts/tab-container/tab-container.js b/js/layouts/tab-container/tab-container.js index 451881d813..ff5b0746b4 100644 --- a/js/layouts/tab-container/tab-container.js +++ b/js/layouts/tab-container/tab-container.js @@ -15,6 +15,7 @@ $(function() css_classes: { is_hidden: 'ibo-is-hidden', + is_disabled: 'ibo-is-disabled', is_transparent: 'ibo-is-transparent', is_opaque: 'ibo-is-opaque', is_scrollable: 'ibo-is-scrollable', @@ -252,6 +253,11 @@ $(function() // Prevent anchor default behaviour oEvent.preventDefault(); + if (oExtraTabTogglerElem.attr('aria-disabled') === true) { + // Corresponding tab is disabled, do nothing + oEvent.preventStopPropagation(); + return; + } // Trigger click event on real tab toggler (the hidden one) const sTargetTabId = oExtraTabTogglerElem.attr('href').replace(/#/, ''); this.element.find(this.js_selectors.tab_header+'[data-tab-id="'+sTargetTabId+'"] '+this.js_selectors.tab_toggler).trigger('click'); @@ -297,21 +303,30 @@ $(function() const sTabId = oTabHeaderElem.attr('data-tab-id'); const oMatchingExtraTabElem = this.element.find(this.js_selectors.extra_tab_toggler+'[href="#'+sTabId+'"]'); - // Disabled tabs are never added to the ExtraTabs list + // Disabled tabs should be disabled in the ExtraTabs list as well + let bIsDisabled = false; if (oTabHeaderElem.attr('aria-disabled') == 'true') { - bIsVisible = true; + bIsDisabled = true; } // Manually check if the tab header is visible if the info isn't passed if (bIsVisible === null) { - bIsVisible = CombodoGlobalToolbox.IsElementVisibleToTheUser(oTabHeaderElem[0], true, 2); - } + bIsVisible = CombodoGlobalToolbox.IsElementVisibleToTheUser(oTabHeaderElem[0], true, 2); + } // Hide/show the corresponding extra tab element - if (bIsVisible) { - oMatchingExtraTabElem.addClass(this.css_classes.is_hidden); - } else { - oMatchingExtraTabElem.removeClass(this.css_classes.is_hidden); - } + if (bIsVisible) { + oMatchingExtraTabElem.addClass(this.css_classes.is_hidden); + } else { + oMatchingExtraTabElem.removeClass(this.css_classes.is_hidden); + } + // Enable/disable the corresponding extra tab element + if (bIsDisabled) { + oMatchingExtraTabElem.attr('aria-disabled', 'true'); + oMatchingExtraTabElem.addClass(this.css_classes.is_disabled); + } else { + oMatchingExtraTabElem.attr('aria-disabled', 'false'); + oMatchingExtraTabElem.removeClass(this.css_classes.is_disabled); + } }, // - Update extra tabs list _updateExtraTabsList: function () { @@ -343,19 +358,21 @@ $(function() return oTabElem.length === 0 ? 0 : oTabElem.prevAll().length; }, - _getTabElementFromTabIndex: function(iIdx) { - return this.element.children(this.js_selectors.tabs_list).children(this.js_selectors.tab_header).eq(iIdx); + _getTabElementFromTabId: function(sId) { + return this.element.children(this.js_selectors.tabs_list).children(this.js_selectors.tab_header+'[data-tab-id="'+sId+'"]'); }, - disableTab: function(iIdx){ - let tabsWidget = this.GetTabsWidget(); + disableTab: function(sId){ + const tabsWidget = this.GetTabsWidget(); + const iIdx = this._getTabIndexFromTabId(sId); tabsWidget.disable(iIdx); - let tabElement = this._getTabElementFromTabIndex(iIdx); - this._updateTabHeaderDisplay(tabElement); + const tabElement = this._getTabElementFromTabId(sId); + this._updateTabHeaderDisplay(tabElement); }, - enableTab: function(iIdx){ - let tabsWidget = this.GetTabsWidget(); + enableTab: function(sId){ + const tabsWidget = this.GetTabsWidget(); + const iIdx = this._getTabIndexFromTabId(sId); tabsWidget.enable(iIdx); - let tabElement = this._getTabElementFromTabIndex(iIdx); + const tabElement = this._getTabElementFromTabId(sId); this._updateTabHeaderDisplay(tabElement); } }); From 94ea8e60e852d0ce1d8385cf03c9d2a031fdfe62 Mon Sep 17 00:00:00 2001 From: "denis.flaven@combodo.com" Date: Thu, 30 Mar 2023 14:29:29 +0200 Subject: [PATCH 3/4] Typo! --- js/layouts/tab-container/tab-container.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/js/layouts/tab-container/tab-container.js b/js/layouts/tab-container/tab-container.js index ff5b0746b4..73b687e384 100644 --- a/js/layouts/tab-container/tab-container.js +++ b/js/layouts/tab-container/tab-container.js @@ -253,9 +253,9 @@ $(function() // Prevent anchor default behaviour oEvent.preventDefault(); - if (oExtraTabTogglerElem.attr('aria-disabled') === true) { + if (oExtraTabTogglerElem.attr('aria-disabled') === 'true') { // Corresponding tab is disabled, do nothing - oEvent.preventStopPropagation(); + oEvent.stopPropagation(); return; } // Trigger click event on real tab toggler (the hidden one) From 477f2f51e9045d4eb8365c0b2fb7b6e775ae9539 Mon Sep 17 00:00:00 2001 From: Molkobain Date: Thu, 30 Mar 2023 16:50:08 +0200 Subject: [PATCH 4/4] Update code to match conventions --- js/layouts/tab-container/scrollable-tabs.js | 4 ++-- js/layouts/tab-container/tab-container.js | 16 +++++++++++++++- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/js/layouts/tab-container/scrollable-tabs.js b/js/layouts/tab-container/scrollable-tabs.js index 91b679e2c5..9ffaec8a03 100644 --- a/js/layouts/tab-container/scrollable-tabs.js +++ b/js/layouts/tab-container/scrollable-tabs.js @@ -380,13 +380,13 @@ $.widget( "itop.scrollabletabs", $.ui.tabs, { // JQuery UI overload disable: function(index){ const panel = this._getPanelForTab( this.tabs[index] ); - panel.css({display: 'none'}); + panel.hide(); this._super( index ); }, // JQuery UI overload enable: function(index) { const panel = this._getPanelForTab( this.tabs[index] ); - panel.css({display: 'block'}); + panel.show(); this._super( index ); }, }); diff --git a/js/layouts/tab-container/tab-container.js b/js/layouts/tab-container/tab-container.js index 73b687e384..a3ba68bbd5 100644 --- a/js/layouts/tab-container/tab-container.js +++ b/js/layouts/tab-container/tab-container.js @@ -305,7 +305,7 @@ $(function() // Disabled tabs should be disabled in the ExtraTabs list as well let bIsDisabled = false; - if (oTabHeaderElem.attr('aria-disabled') == 'true') { + if (oTabHeaderElem.attr('aria-disabled') === 'true') { bIsDisabled = true; } // Manually check if the tab header is visible if the info isn't passed @@ -358,9 +358,19 @@ $(function() return oTabElem.length === 0 ? 0 : oTabElem.prevAll().length; }, + /** + * @param sId {string} The [data-tab-id] of the tab + * @return {Object} The jQuery object representing the tab element + * + * @private + */ _getTabElementFromTabId: function(sId) { return this.element.children(this.js_selectors.tabs_list).children(this.js_selectors.tab_header+'[data-tab-id="'+sId+'"]'); }, + /** + * @param sId {string} The [data-tab-id] of the tab + * @return {Object} The jQuery object representing the tab element + */ disableTab: function(sId){ const tabsWidget = this.GetTabsWidget(); const iIdx = this._getTabIndexFromTabId(sId); @@ -368,6 +378,10 @@ $(function() const tabElement = this._getTabElementFromTabId(sId); this._updateTabHeaderDisplay(tabElement); }, + /** + * @param sId {string} The [data-tab-id] of the tab + * @return {Object} The jQuery object representing the tab element + */ enableTab: function(sId){ const tabsWidget = this.GetTabsWidget(); const iIdx = this._getTabIndexFromTabId(sId);