Hi, Please find attached updated patch. In this patch I have fixed two issues: i. Dialog tab navigation should work even if focus is on footer buttons (Save, Cancel, etc..) ii. Focus should be set to first editable element of dialog when tab cycle goes through all editable footer buttons.
-- *Harshal Dhumal* *Sr. Software Engineer* EnterpriseDB India: http://www.enterprisedb.com The Enterprise PostgreSQL Company On Thu, Jan 10, 2019 at 1:16 PM Harshal Dhumal < harshal.dhu...@enterprisedb.com> wrote: > Hi, > This patch fixes Dialog tabset keyboard navigation. > This regression was caused due to bootstrap 4 changes. > Also I have added jasmine test cases for the same > > > -- > *Harshal Dhumal* > *Sr. Software Engineer* > > EnterpriseDB India: http://www.enterprisedb.com > The Enterprise PostgreSQL Company >
diff --git a/web/pgadmin/browser/static/js/keyboard.js b/web/pgadmin/browser/static/js/keyboard.js index 79266bf..4d9b6ae 100644 --- a/web/pgadmin/browser/static/js/keyboard.js +++ b/web/pgadmin/browser/static/js/keyboard.js @@ -349,11 +349,11 @@ _.extend(pgBrowser.keyboardNavigation, { d: selectedTreeNodeData, }; }, - getDialogTabNavigator: function(dialog) { + getDialogTabNavigator: function(dialogContainer) { const backward_shortcut = pgBrowser.get_preference('browser', 'dialog_tab_backward').value; const forward_shortcut = pgBrowser.get_preference('browser', 'dialog_tab_forward').value; - return new dialogTabNavigator.dialogTabNavigator(dialog, backward_shortcut, forward_shortcut); + return new dialogTabNavigator.dialogTabNavigator(dialogContainer, backward_shortcut, forward_shortcut); }, }); diff --git a/web/pgadmin/browser/static/js/node.js b/web/pgadmin/browser/static/js/node.js index f5cb8cd..30605dd 100644 --- a/web/pgadmin/browser/static/js/node.js +++ b/web/pgadmin/browser/static/js/node.js @@ -412,8 +412,6 @@ define('pgadmin.browser.node', [ view.render(); setFocusOnEl(); newModel.startNewSession(); - // var dialogTabNavigator = pgBrowser.keyboardNavigation.getDialogTabNavigator(view); - pgBrowser.keyboardNavigation.getDialogTabNavigator(view); }, error: function(xhr, error, message) { var _label = that && item ? @@ -450,8 +448,6 @@ define('pgadmin.browser.node', [ view.render(); setFocusOnEl(); newModel.startNewSession(); - // var dialogTabNavigator = pgBrowser.keyboardNavigation.getDialogTabNavigator(view); - pgBrowser.keyboardNavigation.getDialogTabNavigator(view); } } @@ -1083,7 +1079,7 @@ define('pgadmin.browser.node', [ // All buttons will be created within a single // div area. var btnGroup = - $('<div></div>').addClass( + $('<div tabindex="0"></div>').addClass( 'pg-prop-btn-group' ), // Template used for creating a button @@ -1200,7 +1196,6 @@ define('pgadmin.browser.node', [ }); }, }); - createButtons(buttons, 'header', 'pg-prop-btn-group-above'); } j.append(content); @@ -1392,7 +1387,7 @@ define('pgadmin.browser.node', [ ); // Create proper buttons - createButtons([{ + let btn_grp = createButtons([{ label: '', type: 'help', tooltip: gettext('SQL help for this object type.'), @@ -1458,6 +1453,18 @@ define('pgadmin.browser.node', [ }); }, }], 'footer', 'pg-prop-btn-group-below'); + + btn_grp.on('keydown', 'button', function(event) { + if (event.keyCode == 9 && $(this).nextAll('button:not([disabled])').length == 0) { + // set focus back to first editable input element of current active tab once we cycle through all enabled buttons. + commonUtils.findAndSetFocus(view.$el.find('.tab-content div.active')); + return false; + } + }); + + setTimeout(function() { + pgBrowser.keyboardNavigation.getDialogTabNavigator(panel.pgElContainer); + }, 200); } // Create status bar. diff --git a/web/pgadmin/static/js/dialog_tab_navigator.js b/web/pgadmin/static/js/dialog_tab_navigator.js index 19b2045..4472172 100644 --- a/web/pgadmin/static/js/dialog_tab_navigator.js +++ b/web/pgadmin/static/js/dialog_tab_navigator.js @@ -13,13 +13,13 @@ import { findAndSetFocus } from './utils'; import { parseShortcutValue } from './utils'; class dialogTabNavigator { - constructor(dialog, backwardShortcut, forwardShortcut) { + constructor(dialogContainer, backwardShortcut, forwardShortcut) { - this.dialog = dialog; + this.dialogContainer = dialogContainer; this.tabSwitching = false; - this.tabs = this.dialog.$el.find('.nav-tabs'); + this.tabs = this.dialogContainer.find('.nav-tabs'); if (this.tabs.length > 0 ) { this.tabs = this.tabs[0]; @@ -28,13 +28,13 @@ class dialogTabNavigator { this.dialogTabBackward = parseShortcutValue(backwardShortcut); this.dialogTabForward = parseShortcutValue(forwardShortcut); - Mousetrap(this.dialog.el).bind(this.dialogTabBackward, this.onKeyboardEvent.bind(this)); - Mousetrap(this.dialog.el).bind(this.dialogTabForward, this.onKeyboardEvent.bind(this)); + Mousetrap(this.dialogContainer[0]).bind(this.dialogTabBackward, this.onKeyboardEvent.bind(this)); + Mousetrap(this.dialogContainer[0]).bind(this.dialogTabForward, this.onKeyboardEvent.bind(this)); } onKeyboardEvent(event, shortcut) { - var currentTabPane = this.dialog.$el + var currentTabPane = this.dialogContainer .find('.tab-content:first > .tab-pane.active:first'), childTabData = this.isActivePaneHasChildTabs(currentTabPane); @@ -86,7 +86,7 @@ class dialogTabNavigator { var self = this, nextTabPane, innerTabContainer, - prevtab = $(tabs).find('li.active').prev('li'); + prevtab = $(tabs).find('li').has('a.active').prev('li'); if (prevtab.length > 0) { prevtab.find('a').tab('show'); @@ -116,7 +116,7 @@ class dialogTabNavigator { var self = this, nextTabPane, innerTabContainer, - nexttab = $(tabs).find('li.active').next('li'); + nexttab = $(tabs).find('li').has('a.active').next('li'); if(nexttab.length > 0) { nexttab.find('a').tab('show'); @@ -142,11 +142,11 @@ class dialogTabNavigator { } detach() { - Mousetrap(this.dialog.el).unbind(this.dialogTabBackward); - Mousetrap(this.dialog.el).unbind(this.dialogTabForward); + Mousetrap(this.dialogContainer[0]).unbind(this.dialogTabBackward); + Mousetrap(this.dialogContainer[0]).unbind(this.dialogTabForward); } } module.exports = { dialogTabNavigator: dialogTabNavigator, -}; \ No newline at end of file +}; diff --git a/web/pgadmin/tools/import_export/static/js/import_export.js b/web/pgadmin/tools/import_export/static/js/import_export.js index e3957c2..53c788b 100644 --- a/web/pgadmin/tools/import_export/static/js/import_export.js +++ b/web/pgadmin/tools/import_export/static/js/import_export.js @@ -657,10 +657,11 @@ Backform, commonUtils, supportedNodes }); view.$el.attr('tabindex', -1); - // var dialogTabNavigator = pgBrowser.keyboardNavigation.getDialogTabNavigator(view); - pgBrowser.keyboardNavigation.getDialogTabNavigator(view); var container = view.$el.find('.tab-content:first > .tab-pane.active:first'); commonUtils.findAndSetFocus(container); + setTimeout(function() { + pgBrowser.keyboardNavigation.getDialogTabNavigator($(self.elements.dialog)); + }, 200); }, }; }); diff --git a/web/regression/javascript/dialog_tab_navigator_spec.js b/web/regression/javascript/dialog_tab_navigator_spec.js index f355e88..3c6301a 100644 --- a/web/regression/javascript/dialog_tab_navigator_spec.js +++ b/web/regression/javascript/dialog_tab_navigator_spec.js @@ -14,10 +14,10 @@ describe('dialogTabNavigator', function () { let dialog, tabNavigator, backward_shortcut, forward_shortcut; beforeEach(() => { - let dialogHtml =$('<div tabindex="1" class="backform-tab" role="tabpanel">'+ + dialog = $('<div tabindex="1" class="backform-tab" role="tabpanel">'+ ' <ul class="nav nav-tabs" role="tablist">'+ - ' <li role="presentation" class="active">'+ - ' <a data-toggle="tab" tabindex="-1" data-tab-index="1" href="#1" aria-controls="1"> General</a>'+ + ' <li role="presentation">'+ + ' <a class="active" data-toggle="tab" tabindex="-1" data-tab-index="1" href="#1" aria-controls="1"> General</a>'+ ' </li>'+ ' <li role="presentation">'+ ' <a data-toggle="tab" tabindex="-1" data-tab-index="5" href="#2" aria-controls="2"> Default Privileges</a>'+ @@ -52,11 +52,6 @@ describe('dialogTabNavigator', function () { ' </ul>'+ '</div>'); - dialog = {}; - - dialog.el = dialogHtml[0]; - dialog.$el = dialogHtml; - backward_shortcut = { 'alt': false, 'shift': true, @@ -112,4 +107,93 @@ describe('dialogTabNavigator', function () { }); -}); \ No newline at end of file + + describe('navigateForward from fist tab to second tab', function () { + var navigateForwardResult; + beforeEach(() => { + spyOn(tabNavigator, 'navigateForward').and.callThrough(); + + navigateForwardResult = tabNavigator.navigateForward( + dialog.find('ul.nav-tabs:first'), + dialog.find('div#1') + ); + }); + + it('should return true', function () { + + expect(navigateForwardResult).toEqual(true); + + }); + + }); + + + describe('navigateForward from last tab', function () { + var navigateForwardResult; + beforeEach(() => { + + // set second tab active + dialog.find('ul.nav-tabs li a.active').removeClass('active'); + + dialog.find('ul.nav-tabs li a[href="#3"]').addClass('active'); + + spyOn(tabNavigator, 'navigateForward').and.callThrough(); + + navigateForwardResult = tabNavigator.navigateForward( + dialog.find('ul.nav-tabs:first'), + dialog.find('div#1') + ); + }); + + it('should return false', function () { + + expect(navigateForwardResult).toEqual(false); + + }); + + }); + + describe('navigateBackward from second tab to first tab', function () { + var navigateBackwardResult; + beforeEach(() => { + // set second tab active + dialog.find('ul.nav-tabs li a.active').removeClass('active'); + + dialog.find('ul.nav-tabs li a[href="#2"]').addClass('active'); + + spyOn(tabNavigator, 'navigateBackward').and.callThrough(); + + navigateBackwardResult = tabNavigator.navigateBackward( + dialog.find('ul.nav-tabs:first'), + dialog.find('div#1') + ); + }); + + it('should return true', function () { + + expect(navigateBackwardResult).toEqual(true); + + }); + + }); + + describe('navigateBackward from first tab', function () { + var navigateBackwardResult; + beforeEach(() => { + spyOn(tabNavigator, 'navigateBackward').and.callThrough(); + + navigateBackwardResult = tabNavigator.navigateBackward( + dialog.find('ul.nav-tabs:first'), + dialog.find('div#1') + ); + }); + + it('should return false', function () { + + expect(navigateBackwardResult).toEqual(false); + + }); + + }); + +});