Allow navigation of query history using the arrow keys. Fixes #2590

web/pgadmin/feature_tests/query_tool_journey_test.py

@@ -11,6 +11,7 @@ import pyperclip
 import time
 
 from selenium.webdriver import ActionChains
+from selenium.webdriver.common.keys import Keys
 
 from regression.python_test_utils import test_utils
 from regression.feature_utils.base_feature_test import BaseFeatureTest
@@ -75,17 +76,41 @@ class QueryToolJourneyTest(BaseFeatureTest):
         self._execute_query("SELECT * FROM shoes")
 
         self.page.click_tab("History")
-        history_element = self.page.find_by_id("query_list")
-        self.assertIn("SELECT * FROM test_table", history_element.text)
-        self.assertIn("SELECT * FROM shoes", history_element.text)
+        selected_history_entry = self.page.find_by_css_selector("#query_list .selected")
+        self.assertIn("SELECT * FROM shoes", selected_history_entry.text)
+        ActionChains(self.page.driver) \
+            .send_keys(Keys.ARROW_DOWN) \
+            .perform()
+        selected_history_entry = self.page.find_by_css_selector("#query_list .selected")
+        self.assertIn("SELECT * FROM test_table ORDER BY value", selected_history_entry.text)
 
-        history_detail = self.page.find_by_id("query_detail")
-        self.assertIn("SELECT * FROM shoes", history_detail.text)
+        selected_history_detail_pane = self.page.find_by_id("query_detail")
+        self.assertIn("SELECT * FROM test_table ORDER BY value", selected_history_detail_pane.text)
 
-        old_history_list_element = self.page.find_by_xpath("//*[@id='query_list']/ul/li[2]")
-        self.page.click_element(old_history_list_element)
-        history_detail = self.page.find_by_id("query_detail")
-        self.assertIn("SELECT * FROM test_table", history_detail.text)
+        newly_selected_history_entry = self.page.find_by_xpath("//*[@id='query_list']/ul/li[1]")
+        self.page.click_element(newly_selected_history_entry)
+        selected_history_detail_pane = self.page.find_by_id("query_detail")
+        self.assertIn("SELECT * FROM shoes", selected_history_detail_pane.text)
+
+        self.__clear_query_tool()
+
+        self.page.click_element(editor_input)
+        for _ in range(15):
+            self._execute_query("SELECT * FROM hats")
+
+        self.page.click_tab("History")
+
+        query_we_need_to_scroll_to = self.page.find_by_xpath("//*[@id='query_list']/ul/li[17]")
+
+        self.page.click_element(query_we_need_to_scroll_to)
+        self._assert_not_clickable_because_out_of_view(query_we_need_to_scroll_to)
+
+        for _ in range(17):
+            ActionChains(self.page.driver) \
+                .send_keys(Keys.ARROW_DOWN) \
+                .perform()
+
+        self._assert_clickable(query_we_need_to_scroll_to)
 
     def __clear_query_tool(self):
         self.page.click_element(self.page.find_by_xpath("//*[@id='btn-edit']"))
@@ -104,6 +129,12 @@ class QueryToolJourneyTest(BaseFeatureTest):
         self.page.driver.switch_to_frame(self.page.driver.find_element_by_tag_name("iframe"))
         self.page.find_by_id("btn-flash").click()
 
+    def _assert_clickable(self, element):
+        self.page.click_element(element)
+
+    def _assert_not_clickable_because_out_of_view(self, element):
+        self.assertRaises(self.page.click_element(element))
+
     def after(self):
         self.page.close_query_tool()
         self.page.remove_server(self.server)

web/pgadmin/static/jsx/history/query_history.jsx

@@ -7,7 +7,10 @@
 //
 //////////////////////////////////////////////////////////////
 
+/* eslint-disable react/no-find-dom-node */
+
 import React from 'react';
+import ReactDOM from 'react-dom';
 import SplitPane from 'react-split-pane';
 import QueryHistoryEntry from './query_history_entry';
 import QueryHistoryDetail from './query_history_detail';
@@ -20,6 +23,9 @@ const queryDetailDivStyle = {
   display: 'flex',
 };
 
+const ARROWUP = 38;
+const ARROWDOWN = 40;
+
 export default class QueryHistory extends React.Component {
 
   constructor(props) {
@@ -29,6 +35,9 @@ export default class QueryHistory extends React.Component {
       history: [],
       selectedEntry: 0,
     };
+
+    this.onKeyDownHandler = this.onKeyDownHandler.bind(this);
+    this.navigateUpAndDown = this.navigateUpAndDown.bind(this);
   }
 
   componentWillMount() {
@@ -46,6 +55,17 @@ export default class QueryHistory extends React.Component {
     this.resetCurrentHistoryDetail(this.state.history);
   }
 
+  refocus() {
+    if (this.state.history.length > 0) {
+      this.retrieveSelectedQuery().parentElement.focus();
+    }
+  }
+
+  retrieveSelectedQuery() {
+    return ReactDOM.findDOMNode(this)
+      .getElementsByClassName('selected')[0];
+  }
+
   getCurrentHistoryDetail() {
     return this.state.currentHistoryDetail;
   }
@@ -80,16 +100,88 @@ export default class QueryHistory extends React.Component {
     this.setCurrentHistoryDetail(index, this.state.history);
   }
 
+  isInvisible(element) {
+    return this.isAbovePaneTop(element) || this.isBelowPaneBottom(element);
+  }
+
+  isBelowPaneBottom(element) {
+    const paneElement = ReactDOM.findDOMNode(this).getElementsByClassName('Pane1')[0];
+    return element.getBoundingClientRect().bottom > paneElement.getBoundingClientRect().bottom;
+  }
+
+  isAbovePaneTop(element) {
+    const paneElement = ReactDOM.findDOMNode(this).getElementsByClassName('Pane1')[0];
+    return element.getBoundingClientRect().top < paneElement.getBoundingClientRect().top;
+  }
+
+  navigateUpAndDown(event) {
+    let arrowKeys = [ARROWUP, ARROWDOWN];
+    let key = event.keyCode || event.which;
+    if (arrowKeys.indexOf(key) > -1) {
+      event.preventDefault();
+      this.onKeyDownHandler(event);
+      return false;
+    }
+    return true;
+  }
+
+  onKeyDownHandler(event) {
+    if (this.isArrowDown(event)) {
+      if (!this.isLastEntry()) {
+        let nextEntry = this.state.selectedEntry + 1;
+        this.setCurrentHistoryDetail(nextEntry, this.state.history);
+
+        if (this.isInvisible(this.getEntryFromList(nextEntry))) {
+          this.getEntryFromList(nextEntry).scrollIntoView(false);
+        }
+      }
+    } else if (this.isArrowUp(event)) {
+      if (!this.isFirstEntry()) {
+        let previousEntry = this.state.selectedEntry - 1;
+        this.setCurrentHistoryDetail(previousEntry, this.state.history);
+
+        if (this.isInvisible(this.getEntryFromList(previousEntry))) {
+          this.getEntryFromList(previousEntry).scrollIntoView(true);
+        }
+      }
+    }
+  }
+
+  getEntryFromList(entryIndex) {
+    return ReactDOM.findDOMNode(this).getElementsByClassName('entry')[entryIndex];
+  }
+
+  isFirstEntry() {
+    return this.state.selectedEntry === 0;
+  }
+
+  isLastEntry() {
+    return this.state.selectedEntry === this.state.history.length - 1;
+  }
+
+  isArrowUp(event) {
+    return (event.keyCode || event.which) === ARROWUP;
+  }
+
+  isArrowDown(event) {
+    return (event.keyCode || event.which) === ARROWDOWN;
+  }
+
   render() {
     return (
       <SplitPane defaultSize="50%" split="vertical" pane1Style={queryEntryListDivStyle}
                  pane2Style={queryDetailDivStyle}>
-        <div id='query_list' className="query-history">
+        <div id='query_list'
+             className="query-history">
           <ul>
             {this.retrieveOrderedHistory()
               .map((entry, index) =>
-                <li key={index} className='list-item' onClick={this.onClickHandler.bind(this, index)}>
-                  <QueryHistoryEntry historyEntry={entry} isSelected={index == this.state.selectedEntry}/>
+                <li key={index} className='list-item' tabIndex='0'
+                    onClick={this.onClickHandler.bind(this, index)}
+                    onKeyDown={this.navigateUpAndDown}>
+                  <QueryHistoryEntry
+                    historyEntry={entry}
+                    isSelected={index == this.state.selectedEntry}/>
                 </li>)
               .value()
             }

web/pgadmin/static/scss/pgadmin.scss

@@ -16,5 +16,5 @@ $enable-flex: true;
 
 @import 'alert';
 @import 'alertify.overrides';
-@import 'sqleditor/history';
 @import 'backform.overrides';
+@import 'sqleditor/history';

web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js

@@ -1019,9 +1019,19 @@ define('tools.querytool', [
 
         self.history_collection = new HistoryBundle.HistoryCollection([]);
 
-        var queryHistoryElement = React.createElement(
-          queryHistory.QueryHistory, {historyCollection: self.history_collection});
-        ReactDOM.render(queryHistoryElement, $('#history_grid')[0]);
+        var historyComponent;
+        var historyCollectionReactElement = React.createElement(
+          queryHistory.QueryHistory, {
+            historyCollection: self.history_collection,
+            ref: function(component) {
+              historyComponent = component;
+            },
+          });
+        ReactDOM.render(historyCollectionReactElement, $('#history_grid')[0]);
+
+        self.history_panel.on(wcDocker.EVENT.VISIBILITY_CHANGED, function () {
+          historyComponent.refocus();
+        });
       },
 
       // Callback function for Add New Row button click.
@@ -1801,7 +1811,7 @@ define('tools.querytool', [
           });
         },
 
-        // This is a wrapper to call _render function
+        // This is a wrapper to call_render function
         // We need this because we have separated columns route & result route
         // We need to combine both result here in wrapper before rendering grid
         call_render_after_poll: function(res) {

web/regression/javascript/history/query_history_spec.jsx

@@ -7,170 +7,271 @@
 //
 //////////////////////////////////////////////////////////////
 
+/* eslint-disable react/no-find-dom-node */
+
+import jasmineEnzyme from 'jasmine-enzyme';
 import React from 'react';
+import ReactDOM from 'react-dom';
 import QueryHistory from '../../../pgadmin/static/jsx/history/query_history';
 import QueryHistoryEntry from '../../../pgadmin/static/jsx/history/query_history_entry';
 import QueryHistoryDetail from '../../../pgadmin/static/jsx/history/query_history_detail';
 import HistoryCollection from '../../../pgadmin/static/js/history/history_collection';
-import jasmineEnzyme from 'jasmine-enzyme';
 
 import {mount} from 'enzyme';
 
 describe('QueryHistory', () => {
+  let historyCollection;
   let historyWrapper;
+
   beforeEach(() => {
     jasmineEnzyme();
   });
 
-  describe('on construction, when there is no history', () => {
-    beforeEach(function () {
-      const historyCollection = new HistoryCollection([]);
+  describe('when there is no history', () => {
+    beforeEach(() => {
+      historyCollection = new HistoryCollection([]);
       historyWrapper = mount(<QueryHistory historyCollection={historyCollection}/>);
     });
 
+    describe('when we switch to the query history tab', () => {
+      beforeEach(() => {
+        historyWrapper.node.refocus();
+        spyOn(historyWrapper.node, 'retrieveSelectedQuery');
+      });
+
+      it('does not try to focus on any element', () => {
+        expect(historyWrapper.node.retrieveSelectedQuery).not.toHaveBeenCalled();
+      });
+    });
+
     it('has no entries', (done) => {
       let foundChildren = historyWrapper.find(QueryHistoryEntry);
       expect(foundChildren.length).toBe(0);
       done();
     });
-
     it('nothing is displayed on right panel', (done) => {
       let foundChildren = historyWrapper.find(QueryHistoryDetail);
       expect(foundChildren.length).toBe(1);
       done();
     });
 
-    it('does not error', () => {
-    });
+    it('does not error', () => { });
   });
 
-  describe('when it has history', () => {
-    describe('when two SQL queries were executed', () => {
-      let foundChildren;
-      let queryDetail;
-      let historyCollection;
+  describe('when there is history', () => {
+    let historyObjects;
 
-      beforeEach(() => {
-        const historyObjects = [
-          {
-            query: 'second sql statement',
-            start_time: new Date(2016, 11, 11, 1, 33, 5, 99),
-            status: false,
-            row_affected: 1,
-            total_time: '234 msec',
-            message: 'message from second sql query',
-          },
-          {
-            query: 'first sql statement',
-            start_time: new Date(2017, 5, 3, 14, 3, 15, 150),
-            status: true,
-            row_affected: 12345,
-            total_time: '14 msec',
-            message: 'message from first sql query',
-          },
-        ];
-        historyCollection = new HistoryCollection(historyObjects);
+    beforeEach(function () {
+      historyObjects = [{
+        query: 'first sql statement',
+        start_time: new Date(2017, 5, 3, 14, 3, 15, 150),
+        status: true,
+        row_affected: 12345,
+        total_time: '14 msec',
+        message: 'message from first sql query',
+      }, {
+        query: 'second sql statement',
+        start_time: new Date(2016, 11, 11, 1, 33, 5, 99),
+        status: false,
+        row_affected: 1,
+        total_time: '234 msec',
+        message: 'something important ERROR:  message from second sql query',
+      }];
+      historyCollection = new HistoryCollection(historyObjects);
 
-        historyWrapper = mount(<QueryHistory historyCollection={historyCollection}/>);
+      historyWrapper = mount(<QueryHistory historyCollection={historyCollection}/>);
+    });
 
-        foundChildren = historyWrapper.find(QueryHistoryEntry);
-        queryDetail = historyWrapper.find(QueryHistoryDetail);
-      });
+    describe('when all query entries are visible in the pane', () => {
+      describe('when two SQL queries were executed', () => {
+        let foundChildren;
+        let queryDetail;
 
-      describe('the main pane', () => {
-        it('has two query history entries', () => {
-          expect(foundChildren.length).toBe(2);
-        });
-
-        it('displays the query history entries in order', () => {
-          expect(foundChildren.at(0).text()).toContain('first sql statement');
-          expect(foundChildren.at(1).text()).toContain('second sql statement');
-        });
-
-        it('displays the formatted timestamp of the queries in chronological order by most recent first', () => {
-          expect(foundChildren.at(0).text()).toContain('Jun 3 2017 – 14:03:15');
-          expect(foundChildren.at(1).text()).toContain('Dec 11 2016 – 01:33:05');
-        });
-
-        it('renders the most recent query as selected', () => {
-          expect(foundChildren.at(0).nodes.length).toBe(1);
-          expect(foundChildren.at(0).hasClass('selected')).toBeTruthy();
-        });
-
-        it('renders the older query as not selected', () => {
-          expect(foundChildren.at(1).nodes.length).toBe(1);
-          expect(foundChildren.at(1).hasClass('selected')).toBeFalsy();
-          expect(foundChildren.at(1).hasClass('error')).toBeTruthy();
-        });
-      });
-
-      describe('the details pane', () => {
-        it('displays the formatted timestamp', () => {
-          expect(queryDetail.at(0).text()).toContain('6-3-17 14:03:15Date');
-        });
-
-        it('displays the number of rows affected', () => {
-          if (/PhantomJS/.test(window.navigator.userAgent)) {
-            expect(queryDetail.at(0).text()).toContain('12345Rows Affected');
-          } else {
-            expect(queryDetail.at(0).text()).toContain('12,345Rows Affected');
-          }
-        });
-
-        it('displays the total time', () => {
-          expect(queryDetail.at(0).text()).toContain('14 msecDuration');
-        });
-
-        it('displays the full message', () => {
-          expect(queryDetail.at(0).text()).toContain('message from first sql query');
-        });
-
-        it('displays first query SQL', (done) => {
-          setTimeout(() => {
-            expect(queryDetail.at(0).text()).toContain('first sql statement');
-            done();
-          }, 1000);
-        });
-      });
-
-      describe('when the older query is clicked on', () => {
         beforeEach(() => {
-          foundChildren.at(1).simulate('click');
+          spyOn(historyWrapper.node, 'isInvisible').and.returnValue(false);
+          foundChildren = historyWrapper.find(QueryHistoryEntry);
+
+          queryDetail = historyWrapper.find(QueryHistoryDetail);
         });
 
-        it('displays the query in the right pane', () => {
-          expect(queryDetail.at(0).text()).toContain('second sql statement');
-        });
+        describe('the main pane', () => {
+          it('has two query history entries', () => {
+            expect(foundChildren.length).toBe(2);
+          });
 
-        it('renders the most recent query as selected in the left pane', () => {
-          expect(foundChildren.at(0).nodes.length).toBe(1);
-          expect(foundChildren.at(0).hasClass('selected')).toBeFalsy();
-          expect(foundChildren.at(0).hasClass('error')).toBeFalsy();
-        });
+          it('displays the query history entries in order', () => {
+            expect(foundChildren.at(0).text()).toContain('first sql statement');
+            expect(foundChildren.at(1).text()).toContain('second sql statement');
+          });
 
-        it('renders the older query as selected in the left pane', () => {
-          expect(foundChildren.at(1).nodes.length).toBe(1);
-          expect(foundChildren.at(1).hasClass('selected')).toBeTruthy();
-          expect(foundChildren.at(1).hasClass('error')).toBeTruthy();
-        });
-      });
+          it('displays the formatted timestamp of the queries in chronological order by most recent first', () => {
+            expect(foundChildren.at(0).text()).toContain('Jun 3 2017 – 14:03:15');
+            expect(foundChildren.at(1).text()).toContain('Dec 11 2016 – 01:33:05');
+          });
 
-      describe('when a third SQL query is executed', () => {
-        beforeEach(() => {
-          historyCollection.add({
-            query: 'third sql statement',
-            start_time: new Date(2017, 11, 11, 1, 33, 5, 99),
-            status: false,
-            row_affected: 5,
-            total_time: '26 msec',
-            message: 'third sql message',
+          it('renders the most recent query as selected', () => {
+            expect(foundChildren.at(0).nodes.length).toBe(1);
+            expect(foundChildren.at(0).hasClass('selected')).toBeTruthy();
+          });
+
+          it('renders the older query as not selected', () => {
+            expect(foundChildren.at(1).nodes.length).toBe(1);
+            expect(foundChildren.at(1).hasClass('selected')).toBeFalsy();
+            expect(foundChildren.at(1).hasClass('error')).toBeTruthy();
+          });
+
+          describe('when the selected query is the most recent', () => {
+            describe('when we press arrow down', () => {
+              beforeEach(() => {
+                pressArrowDownKey(foundChildren.parent().at(0));
+              });
+
+              it('should select the next query', () => {
+                expect(foundChildren.at(1).nodes.length).toBe(1);
+                expect(foundChildren.at(1).hasClass('selected')).toBeTruthy();
+              });
+
+              it('should display the corresponding detail on the right pane', () => {
+                expect(queryDetail.at(0).text()).toContain('message from second sql query');
+              });
+
+              describe('when arrow down pressed again', () => {
+                it('should not change the selected query', () => {
+                  pressArrowDownKey(foundChildren.parent().at(0));
+
+                  expect(foundChildren.at(1).nodes.length).toBe(1);
+                  expect(foundChildren.at(1).hasClass('selected')).toBeTruthy();
+                });
+              });
+
+              describe('when arrow up is pressed', () => {
+                it('should select the most recent query', () => {
+                  pressArrowUpKey(foundChildren.parent().at(0));
+
+                  expect(foundChildren.at(0).nodes.length).toBe(1);
+                  expect(foundChildren.at(0).hasClass('selected')).toBeTruthy();
+                });
+              });
+            });
+
+            describe('when arrow up is pressed', () => {
+              it('should not change the selected query', () => {
+                pressArrowUpKey(foundChildren.parent().at(0));
+                expect(foundChildren.at(0).nodes.length).toBe(1);
+                expect(foundChildren.at(0).hasClass('selected')).toBeTruthy();
+              });
+            });
           });
         });
 
-        it('displays third query SQL in the right pane', () => {
-          expect(queryDetail.at(0).text()).toContain('third sql statement');
+        describe('the details pane', () => {
+          it('displays the formatted timestamp', () => {
+            expect(queryDetail.at(0).text()).toContain('6-3-17 14:03:15Date');
+          });
+
+          it('displays the number of rows affected', () => {
+            if (/PhantomJS/.test(window.navigator.userAgent)) {
+              expect(queryDetail.at(0).text()).toContain('12345Rows Affected');
+            } else {
+              expect(queryDetail.at(0).text()).toContain('12,345Rows Affected');
+            }
+          });
+
+          it('displays the total time', () => {
+            expect(queryDetail.at(0).text()).toContain('14 msecDuration');
+          });
+
+          it('displays the full message', () => {
+            expect(queryDetail.at(0).text()).toContain('message from first sql query');
+          });
+
+          it('displays first query SQL', (done) => {
+            setTimeout(() => {
+              expect(queryDetail.at(0).text()).toContain('first sql statement');
+              done();
+            }, 1000);
+          });
+        });
+
+        describe('when the older query is clicked on', () => {
+          beforeEach(() => {
+            foundChildren.at(1).simulate('click');
+          });
+
+          it('displays the query in the right pane', () => {
+            expect(queryDetail.at(0).text()).toContain('second sql statement');
+          });
+
+          it('renders the most recent query as selected in the left pane', () => {
+            expect(foundChildren.at(0).nodes.length).toBe(1);
+            expect(foundChildren.at(0).hasClass('selected')).toBeFalsy();
+            expect(foundChildren.at(0).hasClass('error')).toBeFalsy();
+          });
+
+          it('renders the older query as selected in the left pane', () => {
+            expect(foundChildren.at(1).nodes.length).toBe(1);
+            expect(foundChildren.at(1).hasClass('selected')).toBeTruthy();
+            expect(foundChildren.at(1).hasClass('error')).toBeTruthy();
+          });
+        });
+
+        describe('when a third SQL query is executed', () => {
+          beforeEach(() => {
+            historyCollection.add({
+              query: 'third sql statement',
+              start_time: new Date(2017, 11, 11, 1, 33, 5, 99),
+              status: false,
+              row_affected: 5,
+              total_time: '26 msec',
+              message: 'third sql message',
+            });
+
+            foundChildren = historyWrapper.find(QueryHistoryEntry);
+          });
+
+          it('displays third query SQL in the right pane', () => {
+            expect(queryDetail.at(0).text()).toContain('third sql statement');
+          });
+        });
+      });
+    });
+
+    describe('when the first query is outside the visible area', () => {
+      beforeEach(() => {
+        spyOn(historyWrapper.node, 'isInvisible').and.callFake((element) => {
+          return element.textContent.contains('first sql statement');
+        });
+      });
+
+      describe('when the first query is the selected query', () => {
+        describe('when refocus function is called', () => {
+          let selectedListItem;
+
+          beforeEach(() => {
+            selectedListItem = ReactDOM.findDOMNode(historyWrapper.node)
+              .getElementsByClassName('selected')[0].parentElement;
+
+            spyOn(selectedListItem, 'focus');
+            historyWrapper.node.refocus();
+          });
+
+          it('the first query scrolls into view', function () {
+            expect(selectedListItem.focus).toHaveBeenCalledTimes(1);
+          });
         });
       });
     });
   });
-});
+
+  function pressArrowUpKey(node) {
+    pressKey(node, 38);
+  }
+
+  function pressArrowDownKey(node) {
+    pressKey(node, 40);
+  }
+
+  function pressKey(node, keyCode) {
+    node.simulate('keyDown', {keyCode: keyCode});
+  }
+});