From 21bfcd83f420000ba06f146ed1d2010fd66db0ed Mon Sep 17 00:00:00 2001 From: Matthew Kleiman Date: Thu, 20 Jul 2017 20:47:40 +0100 Subject: [PATCH] Allow navigation of query history using the arrow keys. Fixes #2590 --- .../feature_tests/query_tool_journey_test.py | 49 ++- .../static/jsx/history/query_history.jsx | 98 ++++- web/pgadmin/static/scss/pgadmin.scss | 2 +- .../templates/sqleditor/js/sqleditor.js | 18 +- .../javascript/history/query_history_spec.jsx | 341 ++++++++++++------ 5 files changed, 371 insertions(+), 137 deletions(-) diff --git a/web/pgadmin/feature_tests/query_tool_journey_test.py b/web/pgadmin/feature_tests/query_tool_journey_test.py index 56345d7a8..905e08550 100644 --- a/web/pgadmin/feature_tests/query_tool_journey_test.py +++ b/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) diff --git a/web/pgadmin/static/jsx/history/query_history.jsx b/web/pgadmin/static/jsx/history/query_history.jsx index 52806a637..a276e5398 100644 --- a/web/pgadmin/static/jsx/history/query_history.jsx +++ b/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 ( -
+
    {this.retrieveOrderedHistory() .map((entry, index) => -
  • - +
  • +
    • ) .value() } diff --git a/web/pgadmin/static/scss/pgadmin.scss b/web/pgadmin/static/scss/pgadmin.scss index 1595405a5..42f04d2a9 100644 --- a/web/pgadmin/static/scss/pgadmin.scss +++ b/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'; diff --git a/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js b/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js index b3ce2f027..1ebe2b22b 100644 --- a/web/pgadmin/tools/sqleditor/templates/sqleditor/js/sqleditor.js +++ b/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) { diff --git a/web/regression/javascript/history/query_history_spec.jsx b/web/regression/javascript/history/query_history_spec.jsx index c39b9a1ed..a60aeb1e9 100644 --- a/web/regression/javascript/history/query_history_spec.jsx +++ b/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(); }); + 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(); + historyWrapper = mount(); + }); - 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); + }); }); }); }); }); -}); \ No newline at end of file + + function pressArrowUpKey(node) { + pressKey(node, 38); + } + + function pressArrowDownKey(node) { + pressKey(node, 40); + } + + function pressKey(node, keyCode) { + node.simulate('keyDown', {keyCode: keyCode}); + } +});