Subject: [pgAdmin4] [RM3862] Fix Dialog tabset keyboard navigation

From: Harshal Dhumal <[email-masked]>
Date: Thu, 10 Jan 2019 13:16:53 +0530

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/static/js/dialog_tab_navigator.js b/web/pgadmin/static/js/dialog_tab_navigator.js
index 19b2045..ea5fec3 100644
--- a/web/pgadmin/static/js/dialog_tab_navigator.js
+++ b/web/pgadmin/static/js/dialog_tab_navigator.js
@@ -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');
@@ -149,4 +149,4 @@ class dialogTabNavigator {
 
 module.exports = {
   dialogTabNavigator: dialogTabNavigator,
-};
\ No newline at end of file
+};
diff --git a/web/regression/javascript/dialog_tab_navigator_spec.js b/web/regression/javascript/dialog_tab_navigator_spec.js
index f355e88..5c46996 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">'+
+    let dialogHtml = $('<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>'+
@@ -112,4 +112,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.$el.find('ul.nav-tabs:first'),
+        dialog.$el.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.$el.find('ul.nav-tabs li a.active').removeClass('active');
+
+      dialog.$el.find('ul.nav-tabs li a[href="#3"]').addClass('active');
+
+      spyOn(tabNavigator, 'navigateForward').and.callThrough();
+
+      navigateForwardResult = tabNavigator.navigateForward(
+        dialog.$el.find('ul.nav-tabs:first'),
+        dialog.$el.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.$el.find('ul.nav-tabs li a.active').removeClass('active');
+
+      dialog.$el.find('ul.nav-tabs li a[href="#2"]').addClass('active');
+
+      spyOn(tabNavigator, 'navigateBackward').and.callThrough();
+
+      navigateBackwardResult = tabNavigator.navigateBackward(
+        dialog.$el.find('ul.nav-tabs:first'),
+        dialog.$el.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.$el.find('ul.nav-tabs:first'),
+        dialog.$el.find('div#1')
+      );
+    });
+
+    it('should return false', function () {
+
+      expect(navigateBackwardResult).toEqual(false);
+
+    });
+
+  });
+
+});

---

From: Harshal Dhumal <[email-masked]>
Date: Wed, 16 Jan 2019 12:54:45 +0530

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*

> Also I have added jasmine test cases for the same
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);

 
     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(() => {
+    dialog = $('<div tabindex="1" class="backform-tab" role="tabpanel">'+
@@ -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 () {
 
   });
 
-});
+        dialog.find('ul.nav-tabs:first'),
+        dialog.find('div#1')
+      dialog.find('ul.nav-tabs li a.active').removeClass('active');
+      dialog.find('ul.nav-tabs li a[href="#3"]').addClass('active');
+        dialog.find('ul.nav-tabs:first'),
+        dialog.find('div#1')
+      dialog.find('ul.nav-tabs li a.active').removeClass('active');
+      dialog.find('ul.nav-tabs li a[href="#2"]').addClass('active');
+        dialog.find('ul.nav-tabs:first'),
+        dialog.find('div#1')
+        dialog.find('ul.nav-tabs:first'),
+        dialog.find('div#1')

---

From: Akshay Joshi <[email-masked]>
Date: Mon, 21 Jan 2019 16:39:32 +0530

Hi Khushboo

Can you please review it.

-- 
*Akshay Joshi*

*Sr. Software Architect *

*Phone: +91 20-3058-9517Mobile: +91 976-788-8246*

---

From: Khushboo Vashi <[email-masked]>
Date: Tue, 22 Jan 2019 14:51:29 +0530

The patch looks good to me.

On Mon, Jan 21, 2019 at 4:39 PM Akshay Joshi <[email-masked]>
wrote:

>> 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*
>>

---

From: Akshay Joshi <[email-masked]>
Date: Tue, 22 Jan 2019 16:29:03 +0530

Thanks patch applied.

On Tue, Jan 22, 2019 at 2:51 PM Khushboo Vashi <
[email-masked]> wrote:

> On Mon, Jan 21, 2019 at 4:39 PM Akshay Joshi <
> [email-masked]> wrote:
>
>> Hi Khushboo
>>

-- 
*Akshay Joshi*

---

From: Harshal Dhumal <[email-masked]>
Date: Thu, 24 Jan 2019 16:19:18 +0530

Hi,

Please find attached patch to fix dialog tab navigation in backup and
restore dialog.

Note: If first control in dialog is bootstrap switch then tab and dialog
tab navigation do not work.
This is because bootstrap switch captures all keyboard events and it does
not allow them to propagate /bubble up.
As Khushboo is working on new switch control (RM 3051
<https://redmine.postgresql.org/issues/3051>) I haven't fix this issue as a
part of this patch.

-- 
*Harshal Dhumal*
*Sr. Software Engineer*

>>>> cycle goes through all editable footer buttons.
diff --git a/web/pgadmin/static/js/alertify/dialog_wrapper.js b/web/pgadmin/static/js/alertify/dialog_wrapper.js
index 75452f2..7766380 100644
--- a/web/pgadmin/static/js/alertify/dialog_wrapper.js
+++ b/web/pgadmin/static/js/alertify/dialog_wrapper.js
@@ -8,6 +8,7 @@
 //////////////////////////////////////////////////////////////
 
 import * as commonUtils from '../utils';
+import $ from 'jquery';
 
 export class DialogWrapper {
   constructor(
@@ -53,11 +54,20 @@ export class DialogWrapper {
     return undefined;
   }
 
-  focusOnDialog(dialog) {
-    dialog.$el.attr('tabindex', -1);
-    this.pgBrowser.keyboardNavigation.getDialogTabNavigator(dialog);
-    const container = dialog.$el.find('.tab-content:first > .tab-pane.active:first');
+  focusOnDialog(alertifyDialog) {
+    let backform_tab = $(alertifyDialog.elements.body).find('.backform-tab');
+    backform_tab.attr('tabindex', -1);
+    this.pgBrowser.keyboardNavigation.getDialogTabNavigator($(alertifyDialog.elements.dialog));
+    const container = backform_tab.find('.tab-content:first > .tab-pane.active:first');
     commonUtils.findAndSetFocus(container);
+
+    $(alertifyDialog.elements.footer).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($(alertifyDialog.elements.body).find('.tab-content div.active'));
+        return false;
+      }
+    });
   }
 
   isNodeSelected(selectedTreeNode) {
diff --git a/web/pgadmin/static/js/utils.js b/web/pgadmin/static/js/utils.js
index 86727a1..27b999f 100644
--- a/web/pgadmin/static/js/utils.js
+++ b/web/pgadmin/static/js/utils.js
@@ -28,7 +28,7 @@ export function findAndSetFocus(container) {
 
     if (first_el.length == 0) {
       first_el = container
-        .find('.pgadmin-controls:first>input:enabled,.CodeMirror-scroll');
+        .find('.pgadmin-controls:first input:enabled,.CodeMirror-scroll');
     }
 
     if(first_el.length > 0) {
diff --git a/web/pgadmin/tools/backup/static/js/backup_dialog_wrapper.js b/web/pgadmin/tools/backup/static/js/backup_dialog_wrapper.js
index 931d0a0..a9a01e5 100644
--- a/web/pgadmin/tools/backup/static/js/backup_dialog_wrapper.js
+++ b/web/pgadmin/tools/backup/static/js/backup_dialog_wrapper.js
@@ -99,7 +99,7 @@ export class BackupDialogWrapper extends DialogWrapper {
 
     this.elements.content.appendChild($container.get(0));
 
-    this.focusOnDialog(dialog);
+    this.focusOnDialog(this);
     this.setListenersForFilenameChanges();
   }
 
diff --git a/web/pgadmin/tools/restore/static/js/restore_dialog_wrapper.js b/web/pgadmin/tools/restore/static/js/restore_dialog_wrapper.js
index 0d211ae..aba3048 100644
--- a/web/pgadmin/tools/restore/static/js/restore_dialog_wrapper.js
+++ b/web/pgadmin/tools/restore/static/js/restore_dialog_wrapper.js
@@ -99,7 +99,7 @@ export class RestoreDialogWrapper extends DialogWrapper {
 
     this.elements.content.appendChild($container.get(0));
 
-    this.focusOnDialog(dialog);
+    this.focusOnDialog(this);
     this.setListenersForFilenameChanges();
   }

---

From: Akshay Joshi <[email-masked]>
Date: Thu, 24 Jan 2019 17:19:00 +0530

Thanks patch applied.

On Thu, Jan 24, 2019 at 4:19 PM Harshal Dhumal <
[email-masked]> wrote:

> <https://redmine.postgresql.org/issues/3051>) I haven't fix this issue as
> a part of this patch.
>
> --
> *Harshal Dhumal*
> *Sr. Software Engineer*
>
> On Tue, Jan 22, 2019 at 4:29 PM Akshay Joshi <
>>>>> i. Dialog tab navigation should work even if focus is on footer
>>>>> buttons (Save, Cancel, etc..)

-- 
*Akshay Joshi*