Re: [pgadmin4][PATCH] Query History Arrow Navigation and Styling

[Harshal Dhumal]

Hi,

On Fri, Jul 21, 2017 at 2:24 PM, Dave Page <dp...@pgadmin.org> wrote:

> Hi
>
> On Fri, Jul 21, 2017 at 9:38 AM, Harshal Dhumal <
> harshal.dhu...@enterprisedb.com> wrote:
>
>> Hi,
>>
>> Currently if user clicks on empty white area below history entry then
>> arrow navigation does not work.
>> Attached patch fixes this issue.
>>
>
> This fixes that problem, but stops it working if I *don't* click on the
> white area first. We need both I think :-)
>
Yes we need both. Please find updated patch


>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

diff --git a/web/pgadmin/static/jsx/history/query_history.jsx b/web/pgadmin/static/jsx/history/query_history.jsx
index a276e53..8efe08b 100644
--- a/web/pgadmin/static/jsx/history/query_history.jsx
+++ b/web/pgadmin/static/jsx/history/query_history.jsx
@@ -57,13 +57,13 @@ export default class QueryHistory extends React.Component {
 
   refocus() {
     if (this.state.history.length > 0) {
-      this.retrieveSelectedQuery().parentElement.focus();
+      this.retrieveQueryListPane().focus();
     }
   }
 
-  retrieveSelectedQuery() {
+  retrieveQueryListPane() {
     return ReactDOM.findDOMNode(this)
-      .getElementsByClassName('selected')[0];
+      .getElementsByClassName('query-history')[0];
   }
 
   getCurrentHistoryDetail() {
@@ -172,13 +172,14 @@ export default class QueryHistory extends React.Component {
       <SplitPane defaultSize="50%" split="vertical" pane1Style={queryEntryListDivStyle}
                  pane2Style={queryDetailDivStyle}>
         <div id='query_list'
-             className="query-history">
+             className="query-history"
+             onKeyDown={this.navigateUpAndDown}
+             tabIndex='0'>
           <ul>
             {this.retrieveOrderedHistory()
               .map((entry, index) =>
-                <li key={index} className='list-item' tabIndex='0'
-                    onClick={this.onClickHandler.bind(this, index)}
-                    onKeyDown={this.navigateUpAndDown}>
+                <li key={index} className='list-item'
+                    onClick={this.onClickHandler.bind(this, index)}>
                   <QueryHistoryEntry
                     historyEntry={entry}
                     isSelected={index == this.state.selectedEntry}/>
@@ -198,4 +199,4 @@ QueryHistory.propTypes = {
 
 export {
   QueryHistory,
-};
\ No newline at end of file
+};
diff --git a/web/pgadmin/static/scss/sqleditor/_history.scss b/web/pgadmin/static/scss/sqleditor/_history.scss
index ed0c10f..942af03 100644
--- a/web/pgadmin/static/scss/sqleditor/_history.scss
+++ b/web/pgadmin/static/scss/sqleditor/_history.scss
@@ -1,4 +1,5 @@
 .query-history {
+  height: 100%;
   .list-item {
     border-bottom: 1px solid $color-gray-3;
   }
diff --git a/web/regression/javascript/history/query_history_spec.jsx b/web/regression/javascript/history/query_history_spec.jsx
index 2fa6a7d..c3b09ba 100644
--- a/web/regression/javascript/history/query_history_spec.jsx
+++ b/web/regression/javascript/history/query_history_spec.jsx
@@ -36,11 +36,11 @@ describe('QueryHistory', () => {
     describe('when we switch to the query history tab', () => {
       beforeEach(() => {
         historyWrapper.node.refocus();
-        spyOn(historyWrapper.node, 'retrieveSelectedQuery');
+        spyOn(historyWrapper.node, 'retrieveQueryListPane');
       });
 
       it('does not try to focus on any element', () => {
-        expect(historyWrapper.node.retrieveSelectedQuery).not.toHaveBeenCalled();
+        expect(historyWrapper.node.retrieveQueryListPane).not.toHaveBeenCalled();
       });
     });
 
@@ -49,7 +49,7 @@ describe('QueryHistory', () => {
       expect(foundChildren.length).toBe(0);
       done();
     });
-    
+
     it('nothing is displayed on right panel', (done) => {
       let foundChildren = historyWrapper.find(QueryHistoryDetail);
       expect(foundChildren.length).toBe(1);
@@ -264,7 +264,7 @@ describe('QueryHistory', () => {
 
           beforeEach(() => {
             selectedListItem = ReactDOM.findDOMNode(historyWrapper.node)
-              .getElementsByClassName('selected')[0].parentElement;
+              .getElementsByClassName('query-history')[0];
 
             spyOn(selectedListItem, 'focus');
             historyWrapper.node.refocus();