Subject: [pgAdmin4][RM3908] backgrid navigation fix for Select2Cell and PrivilegeCell

From: Harshal Dhumal <[email-masked]>
Date: Thu, 17 Jan 2019 19:22:34 +0530

Hi,
Please find attached patch to fix backgrid navigation using keyboard
for Select2Cell and PrivilegeCell

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

EnterpriseDB India: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/column/static/js/column.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/column/static/js/column.js
index 9031d94..b00998b 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/column/static/js/column.js
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/column/static/js/column.js
@@ -269,39 +269,7 @@ define('pgadmin.node.column', [
           type: 'text', disabled: 'notInSchema', mode: ['properties'],
         },{
           id: 'cltype', label: gettext('Data type'),
-          cell: Backgrid.Extension.NodeAjaxOptionsCell.extend({
-            exitEditMode: function(e) {
-              var self = this;
-              this.$select.off('blur', this.exitEditMode);
-              this.$select.select2('close');
-              this.$el.removeClass('editor');
-                // Once user have selected a value
-                // we can shift to next cell if it is editable
-              var next_cell, length_cell = this.$el.next(),
-                not_null_cell = this.$el.next().next().next();
-
-                // Add delay so that Select2 cell tab event is captured
-                // first before triggerring backgrid:edited event.
-              setTimeout(function() {
-                  // First check Length column if it is disable then goto
-                  // Not Null column
-                if(length_cell && length_cell.hasClass('editable') && e) {
-                  next_cell = length_cell;
-                } else if(not_null_cell && not_null_cell.hasClass('editable') && e) {
-                  next_cell = not_null_cell;
-                }
-
-                if(next_cell) {
-                  e.preventDefault();
-                  e.stopPropagation();
-                  var command = new Backgrid.Command({key: 'Tab', keyCode: 9, which: 9});
-                  self.model.trigger('backgrid:edited', self.model, self.column,
-                                      command);
-                  next_cell.trigger('focus');
-                }
-              }, 20);
-            },
-          }),
+          cell: Backgrid.Extension.NodeAjaxOptionsCell,
           type: 'text', disabled: 'inSchemaWithColumnCheck',
           control: 'node-ajax-options', url: 'get_types', node: 'table',
           cellHeaderClasses:'width_percent_30', first_empty: true,
diff --git a/web/pgadmin/browser/server_groups/servers/static/js/privilege.js b/web/pgadmin/browser/server_groups/servers/static/js/privilege.js
index c5663f7..5476cf8 100644
--- a/web/pgadmin/browser/server_groups/servers/static/js/privilege.js
+++ b/web/pgadmin/browser/server_groups/servers/static/js/privilege.js
@@ -329,13 +329,13 @@ define(['sources/gettext', 'underscore', 'jquery', 'backbone', 'backform',
         '<tr class="<%= header ? "header" : "" %>">',
         ' <td class="renderable">',
         '  <label class="privilege_label">',
-        '   <input type="checkbox" name="privilege" privilege="<%- privilege_type %>" target="<%- target %>" <%= privilege ? \'checked\' : "" %>></input>',
+        '   <input type="checkbox" tabindex="1" name="privilege" privilege="<%- privilege_type %>" target="<%- target %>" <%= privilege ? \'checked\' : "" %>></input>',
         '   <%- privilege_label %>',
         '  </label>',
         ' </td>',
         ' <td class="renderable">',
         '  <label class="privilege_label">',
-        '   <input type="checkbox" name="with_grant" privilege="<%- privilege_type %>" target="<%- target %>" <%= with_grant ? \'checked\' : "" %> <%= enable_with_grant ? "" : \'disabled\'%>></input>',
+        '   <input type="checkbox" tabindex="1" name="with_grant" privilege="<%- privilege_type %>" target="<%- target %>" <%= with_grant ? \'checked\' : "" %> <%= enable_with_grant ? "" : \'disabled\'%>></input>',
         '   WITH GRANT OPTION',
         '  </label>',
         ' </td>',
@@ -344,6 +344,7 @@ define(['sources/gettext', 'underscore', 'jquery', 'backbone', 'backform',
       events: {
         'change': 'privilegeChanged',
         'blur': 'lostFocus',
+        'keydown': 'lostFocus',
       },
 
       render: function () {
@@ -608,7 +609,42 @@ define(['sources/gettext', 'underscore', 'jquery', 'backbone', 'backform',
               node = node.parentNode;
             }
             return false;
-          };
+          },
+          model = this.model,
+          column = this.column,
+          command = new Backgrid.Command(ev),
+          coll = this.model.get(this.column.get('name'));
+
+        if (command.moveUp() || command.moveDown() || command.save()) {
+            // backgrid vertical navigation (Up/Down arrow key)
+          ev.preventDefault();
+          ev.stopPropagation();
+          model.trigger('backgrid:edited', model, column, command);
+          return;
+        }
+        // esc
+        else if (command.cancel()) {
+          // undo
+          ev.stopPropagation();
+          model.trigger('backgrid:edited', model, column, command);
+          return;
+        } else if (command.moveRight()) {
+          // If we are at the last privilege then we should move to next cell
+          if (coll.last().get('privilege_type') === $(ev.target).attr('privilege')) {
+            if ((ev.target.name === 'privilege' && !ev.target.checked ) ||
+              $(ev.target).attr('name') === 'with_grant') {
+              ev.stopPropagation();
+              model.trigger('backgrid:edited', model, column, command);
+              return;
+            }
+          }
+        } else if (command.moveLeft() && ev.target.name === 'privilege' &&
+          $(ev.target).attr('privilege') === 'ALL') {
+          // If we are at the fist privilege then we should move to previous cell
+          ev.stopPropagation();
+          model.trigger('backgrid:edited', model, column, command);
+          return;
+        }
 
         /*
          * Between leaving the old element focus and entering the new element focus the
@@ -630,7 +666,6 @@ define(['sources/gettext', 'underscore', 'jquery', 'backbone', 'backform',
             var m = self.model;
             m.trigger('backgrid:edited', m, self.column, new Backgrid.Command(ev));
           }},10);
-        return;
       },
     });
 
@@ -700,6 +735,30 @@ define(['sources/gettext', 'underscore', 'jquery', 'backbone', 'backform',
         }
       });
     },
+
+    events: {
+      'click': 'enterEditMode',
+      'keydown': 'saveOrCancel',
+    },
+
+    saveOrCancel: function (e) {
+      var model = this.model;
+      var column = this.column;
+      var command = new Backgrid.Command(e);
+
+      if (command.moveUp() || command.moveDown() || command.moveLeft() || command.moveRight() ||
+        command.save()) {
+        e.preventDefault();
+        e.stopPropagation();
+        model.trigger('backgrid:edited', model, column, command);
+      }
+    // esc
+      else if (command.cancel()) {
+      // undo
+        e.stopPropagation();
+        model.trigger('backgrid:edited', model, column, command);
+      }
+    },
   });
 
   return PrivilegeRoleModel;
diff --git a/web/pgadmin/static/js/backgrid.pgadmin.js b/web/pgadmin/static/js/backgrid.pgadmin.js
index 2832b5a..9ea4ad4 100644
--- a/web/pgadmin/static/js/backgrid.pgadmin.js
+++ b/web/pgadmin/static/js/backgrid.pgadmin.js
@@ -591,11 +591,29 @@ define([
       this.$el.removeClass('editor');
     },
 
+    saveOrCancel: function (e) {
+      var model = this.model;
+      var column = this.column;
+
+      var command = new Backgrid.Command(e);
+      var blurred = e.type === 'blur';
+
+      if (command.moveUp() || command.moveDown() || command.moveLeft() || command.moveRight() ||
+          command.save() || blurred) {
+
+        this.exitEditMode();
+        e.preventDefault();
+        e.stopPropagation();
+        model.trigger('backgrid:edited', model, column, command);
+      }
+    },
     events: {
       'select2:open': 'enterEditMode',
       'select2:close': 'exitEditMode',
       'change': 'onSave',
       'select2:unselect': 'onSave',
+      'blur': 'saveOrCancel',
+      'keydown': 'saveOrCancel',
     },
     /** @property {function(Object, ?Object=): string} template */
     template: _.template([
diff --git a/web/pgadmin/static/scss/_pgadmin.style.scss b/web/pgadmin/static/scss/_pgadmin.style.scss
index 6ec4293..73528b9 100644
--- a/web/pgadmin/static/scss/_pgadmin.style.scss
+++ b/web/pgadmin/static/scss/_pgadmin.style.scss
@@ -653,6 +653,10 @@ fieldset.inline-fieldset > div {
   outline: none;
 }
 
+input[type="checkbox"]:focus {
+    outline: $color-primary auto 5px !important;
+}
+
 div.rolmembership {
     margin-top: 15px;
 }

---

From: Akshay Joshi <[email-masked]>
Date: Mon, 21 Jan 2019 16:39:49 +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 15:02:15 +0530

Hi Harshal,

The Selec2Cell and PrivilegeCell are working properly but I think we should
also consider below:

1. The Backgrid and sub-node should have accessibility including every
button, cell and control.
2. The Backgrid table header is accessible through keyboard but it should
be highlighted when it gets focus.

@Akshay, we can commit this patch but we should create the separate RM for
above fixes.

Thanks,
Khushboo

---

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

Thanks patch applied

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

> Hi Harshal,
>
> The Selec2Cell and PrivilegeCell are working properly but I think we
> should also consider below:
>

     RM # 3919 has been created by Harshal.

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

-- 
*Akshay Joshi*