Hi Dave,

Please find patch attached, There were two issues,
1) We removed the default button to clear the editor window, it
broke _clear_query_tool() functionality.
2) The buttons arrangements, we added new Edit button in between Delete and
Filter button causing the "Explain" -> "Explain Options" sub menu to go out
of browser visibility in feature test, so it was failing.

I have put Edit button near Clear button for now, until we come up with new
design for our editor for displaying these options.


--
Regards,
Murtuza Zabuawala
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Thu, Jul 20, 2017 at 6:04 PM, Murtuza Zabuawala <
murtuza.zabuaw...@enterprisedb.com> wrote:

> Hi Dave,
>
> I am working on this, will send you patch soon.
>
> On Thu, Jul 20, 2017 at 5:53 PM, Dave Page <dp...@pgadmin.org> wrote:
>
>> Did you get a chance to look at this yet Murtuza?
>>
>> On Wed, Jul 19, 2017 at 3:37 PM, Murtuza Zabuawala <
>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>
>>> Sure, Will take a look.
>>>
>>> --
>>> Regards,
>>> Murtuza Zabuawala
>>> EnterpriseDB: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>> On Wed, Jul 19, 2017 at 8:00 PM, Dave Page <dp...@pgadmin.org> wrote:
>>>
>>>> Except I managed to break a couple of tests :-(. Can you take a look
>>>> please? I've had some other work come up that I need to deal with.
>>>>
>>>> ======================================================================
>>>> ERROR: runTest (pgadmin.feature_tests.query_t
>>>> ool_journey_test.QueryToolJourneyTest)
>>>> Tests the path through the query tool
>>>> ----------------------------------------------------------------------
>>>> Traceback (most recent call last):
>>>>   File 
>>>> "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/query_tool_journey_test.py",
>>>> line 45, in runTest
>>>>     self._test_history_tab()
>>>>   File 
>>>> "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/query_tool_journey_test.py",
>>>> line 71, in _test_history_tab
>>>>     self.__clear_query_tool()
>>>>   File 
>>>> "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/query_tool_journey_test.py",
>>>> line 91, in __clear_query_tool
>>>>     self.page.click_element(self.page.find_by_xpath("//*[@id='bt
>>>> n-edit']"))
>>>>   File 
>>>> "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py",
>>>> line 148, in find_by_xpath
>>>>     return self.wait_for_element(lambda driver:
>>>> driver.find_element_by_xpath(xpath))
>>>>   File 
>>>> "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py",
>>>> line 232, in wait_for_element
>>>>     return self._wait_for("element to exist", element_if_it_exists)
>>>>   File 
>>>> "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py",
>>>> line 282, in _wait_for
>>>>     "Timed out waiting for " + waiting_for_message)
>>>>   File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packa
>>>> ges/selenium/webdriver/support/wait.py", line 80, in until
>>>>     raise TimeoutException(message, screen, stacktrace)
>>>> TimeoutException: Message: Timed out waiting for element to exist
>>>>
>>>>
>>>> ======================================================================
>>>> ERROR: runTest (pgadmin.feature_tests.query_t
>>>> ool_tests.QueryToolFeatureTest)
>>>> Query tool feature test
>>>> ----------------------------------------------------------------------
>>>> Traceback (most recent call last):
>>>>   File 
>>>> "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/query_tool_tests.py",
>>>> line 52, in runTest
>>>>     self._clear_query_tool()
>>>>   File 
>>>> "/Users/dpage/git/pgadmin4/web/pgadmin/feature_tests/query_tool_tests.py",
>>>> line 173, in _clear_query_tool
>>>>     self.page.find_by_id("btn-edit").click()
>>>>   File 
>>>> "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py",
>>>> line 151, in find_by_id
>>>>     return self.wait_for_element(lambda driver:
>>>> driver.find_element_by_id(element_id))
>>>>   File 
>>>> "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py",
>>>> line 232, in wait_for_element
>>>>     return self._wait_for("element to exist", element_if_it_exists)
>>>>   File 
>>>> "/Users/dpage/git/pgadmin4/web/regression/feature_utils/pgadmin_page.py",
>>>> line 282, in _wait_for
>>>>     "Timed out waiting for " + waiting_for_message)
>>>>   File "/Users/dpage/.virtualenvs/pgadmin4/lib/python2.7/site-packa
>>>> ges/selenium/webdriver/support/wait.py", line 80, in until
>>>>     raise TimeoutException(message, screen, stacktrace)
>>>> TimeoutException: Message: Timed out waiting for element to exist
>>>>
>>>>
>>>> ----------------------------------------------------------------------
>>>> Ran 9 tests in 262.111s
>>>>
>>>> On Wed, Jul 19, 2017 at 11:55 AM, Murtuza Zabuawala <
>>>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>>>
>>>>> Thank you Dave.
>>>>>
>>>>> On Wed, Jul 19, 2017 at 4:17 PM, Dave Page <dp...@pgadmin.org> wrote:
>>>>>
>>>>>> Thanks, applied.
>>>>>>
>>>>>> I also took the opportunity to tidy up the menus a little and add
>>>>>> access keys for accessibility.
>>>>>>
>>>>>> One change I made was to make the Edit and Clear menus not have a
>>>>>> default option - e.g. instead of a button with a drop-down next to it,
>>>>>> they're now a single dropdown button with icon. I think this works better
>>>>>> as there are no obvious candidates for the "default" option for those
>>>>>> menus. I'm not overly enthusiastic about the look of those buttons 
>>>>>> though,
>>>>>> so if anyone has a better idea how they should be styled, please yelp
>>>>>> (CCing Chethana for his input as well)...
>>>>>>
>>>>>>
>>>>>>
>>>>>> On Wed, Jul 19, 2017 at 9:56 AM, Murtuza Zabuawala <
>>>>>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>>>>>
>>>>>>> Just a FYI,
>>>>>>> You need to run yarn bundle for this to be working as Surinder has
>>>>>>> moved all the CodeMirror code into bundle package.
>>>>>>>
>>>>>>> On Wed, Jul 19, 2017 at 2:20 PM, Murtuza Zabuawala <
>>>>>>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>>>>>>
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> PFA updated patch,
>>>>>>>> 1) Added Keyboard shortcuts to comment line, uncomment line and
>>>>>>>> comment/uncomment block of code also added drop-down for the same.
>>>>>>>> 2) Also added options for indent & unindent code in the same
>>>>>>>> drop-down.
>>>>>>>> 3) Updated shortcut documents accordingly.
>>>>>>>>
>>>>>>>> Please review.
>>>>>>>>
>>>>>>>> On Mon, Jul 17, 2017 at 3:05 PM, Dave Page <dp...@pgadmin.org>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>>> Hi
>>>>>>>>>
>>>>>>>>> On Mon, Jul 17, 2017 at 10:31 AM, Murtuza Zabuawala <
>>>>>>>>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>>>>>>>>
>>>>>>>>>> Hi Dave,
>>>>>>>>>>
>>>>>>>>>> On Mon, Jul 17, 2017 at 2:33 PM, Dave Page <dp...@pgadmin.org>
>>>>>>>>>> wrote:
>>>>>>>>>>
>>>>>>>>>>> Hi
>>>>>>>>>>>
>>>>>>>>>>> On Wed, Jul 12, 2017 at 1:16 PM, Murtuza Zabuawala <
>>>>>>>>>>> murtuza.zabuaw...@enterprisedb.com> wrote:
>>>>>>>>>>>
>>>>>>>>>>>> Hi,
>>>>>>>>>>>>
>>>>>>>>>>>> PFA patch which will add functionality to allow user to
>>>>>>>>>>>> comment/uncomment code in query editor.
>>>>>>>>>>>> RM#2456
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> This is cool, but I'm not sure it's right as-is:
>>>>>>>>>>>
>>>>>>>>>>> * I prefer SQL style commenting, e.g.
>>>>>>>>>>>
>>>>>>>>>>> -- This is a comment
>>>>>>>>>>>
>>>>>>>>>>> Should we make that a config option if CodeMirror can do it? Or
>>>>>>>>>>> a different hotkey?
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>> I'll check the extension code and update you accordingly, and It
>>>>>>>>>> will be good idea to keep the both the options because with large 
>>>>>>>>>> code
>>>>>>>>>> block current style works the best.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Right.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> * You've added it as an option to the Clear XXX dropdown, which
>>>>>>>>>>> really isn't the right place in my opinion. Should we add a new 
>>>>>>>>>>> drop down
>>>>>>>>>>> for this, and include some/all of the other Editing options on 
>>>>>>>>>>> there? E.g.
>>>>>>>>>>> tab/shift-tab.
>>>>>>>>>>>
>>>>>>>>>>> I thought that is misc options dropdown for our editor, but I
>>>>>>>>>> don't see any point adding new drop down for one single option, Can 
>>>>>>>>>> we add
>>>>>>>>>> new button instead?
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> I think you missed this bit:  "and include some/all of the other
>>>>>>>>> Editing options on there? E.g. tab/shift-tab.". Essentially, we'd be 
>>>>>>>>> adding
>>>>>>>>> an Edit menu...
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> * I think the docs should say Ctrl+Shift+/ rather than
>>>>>>>>>>> Shift+Ctrl+/, and be ordered in the table to reflect that. It seems 
>>>>>>>>>>> more
>>>>>>>>>>> natural to me.
>>>>>>>>>>>
>>>>>>>>>>> Initially I wrote ctrl + shift + /only but when I saw all other
>>>>>>>>>> shortcuts starts with Shift , then I changed it to shift + ctrl + / 
>>>>>>>>>> :)
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> No they don't - Ctrl+Alt+Left for example. I believe it's normal
>>>>>>>>> to put Ctrl first, then Shift as it's a modifier.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> Thoughts?
>>>>>>>>>>>
>>>>>>>>>>> --
>>>>>>>>>>> Dave Page
>>>>>>>>>>> Blog: http://pgsnake.blogspot.com
>>>>>>>>>>> Twitter: @pgsnake
>>>>>>>>>>>
>>>>>>>>>>> EnterpriseDB UK: http://www.enterprisedb.com
>>>>>>>>>>> The Enterprise PostgreSQL Company
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> Dave Page
>>>>>>>>> Blog: http://pgsnake.blogspot.com
>>>>>>>>> Twitter: @pgsnake
>>>>>>>>>
>>>>>>>>> EnterpriseDB UK: http://www.enterprisedb.com
>>>>>>>>> The Enterprise PostgreSQL Company
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Dave Page
>>>>>> Blog: http://pgsnake.blogspot.com
>>>>>> Twitter: @pgsnake
>>>>>>
>>>>>> EnterpriseDB UK: http://www.enterprisedb.com
>>>>>> The Enterprise PostgreSQL Company
>>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>> --
>>>> Dave Page
>>>> Blog: http://pgsnake.blogspot.com
>>>> Twitter: @pgsnake
>>>>
>>>> EnterpriseDB UK: http://www.enterprisedb.com
>>>> The Enterprise PostgreSQL Company
>>>>
>>>
>>>
>>
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>
>
diff --git a/web/pgadmin/feature_tests/query_tool_journey_test.py 
b/web/pgadmin/feature_tests/query_tool_journey_test.py
index 56345d7..e283958 100644
--- a/web/pgadmin/feature_tests/query_tool_journey_test.py
+++ b/web/pgadmin/feature_tests/query_tool_journey_test.py
@@ -88,7 +88,12 @@ class QueryToolJourneyTest(BaseFeatureTest):
         self.assertIn("SELECT * FROM test_table", history_detail.text)
 
     def __clear_query_tool(self):
-        self.page.click_element(self.page.find_by_xpath("//*[@id='btn-edit']"))
+        
self.page.click_element(self.page.find_by_xpath("//*[@id='btn-clear-dropdown']"))
+        ActionChains(self.driver) \
+            .move_to_element(self.page.find_by_xpath("//*[@id='btn-clear']")) \
+            .perform()
+        
self.page.click_element(self.page.find_by_xpath("//*[@id='btn-clear']"))
+        time.sleep(1)
         self.page.click_modal('Yes')
 
     def _navigate_to_query_tool(self):
diff --git a/web/pgadmin/feature_tests/query_tool_tests.py 
b/web/pgadmin/feature_tests/query_tool_tests.py
index 6327ad2..0130967 100644
--- a/web/pgadmin/feature_tests/query_tool_tests.py
+++ b/web/pgadmin/feature_tests/query_tool_tests.py
@@ -169,14 +169,13 @@ class QueryToolFeatureTest(BaseFeatureTest):
         self.page.toggle_open_tree_item('acceptance_test_db')
 
     def _clear_query_tool(self):
-        # clear codemirror.
-        self.page.find_by_id("btn-edit").click()
-        # wait for alertify dialog open animation to complete.
-        time.sleep(1)
-
-        
self.page.click_element(self.page.find_by_xpath("//button[contains(.,'Yes')]"))
-        # wait for alertify dialog close animation to complete.
+        
self.page.click_element(self.page.find_by_xpath("//*[@id='btn-clear-dropdown']"))
+        ActionChains(self.driver) \
+            .move_to_element(self.page.find_by_xpath("//*[@id='btn-clear']")) \
+            .perform()
+        
self.page.click_element(self.page.find_by_xpath("//*[@id='btn-clear']"))
         time.sleep(1)
+        self.page.click_modal('Yes')
 
     def _on_demand_result(self):
         ON_DEMAND_CHUNKS = 2
@@ -314,24 +313,16 @@ SELECT generate_series(1, 1000) as id order by id desc"""
 SELECT generate_series(1, 1000) as id order by id desc"""
 
         wait = WebDriverWait(self.page.driver, 10)
-
         self.page.fill_codemirror_area_with(query)
-
         query_op = self.page.find_by_id("btn-query-dropdown")
         query_op.click()
-
         ActionChains(self.driver).move_to_element(
             query_op.find_element_by_xpath(
                 "//li[contains(.,'Explain Options')]")).perform()
-
         self.page.find_by_id("btn-explain-verbose").click()
-
         self.page.find_by_id("btn-explain").click()
-
         self.page.wait_for_query_tool_loading_indicator_to_disappear()
-
         self.page.click_tab('Data Output')
-
         canvas = wait.until(EC.presence_of_element_located(
             (By.CSS_SELECTOR, "#datagrid .slick-viewport .grid-canvas"))
         )
diff --git a/web/pgadmin/tools/datagrid/templates/datagrid/index.html 
b/web/pgadmin/tools/datagrid/templates/datagrid/index.html
index cb70052..9ad02f9 100644
--- a/web/pgadmin/tools/datagrid/templates/datagrid/index.html
+++ b/web/pgadmin/tools/datagrid/templates/datagrid/index.html
@@ -112,33 +112,6 @@
                 </button>
             </div>
             <div class="btn-group" role="group" aria-label="">
-                <button id="btn-edit-dropdown" type="button" class="btn 
btn-default dropdown-toggle"
-                        data-toggle="dropdown" aria-haspopup="true" 
aria-expanded="false"
-                        title="{{ _('Edit') }}" accesskey="e">
-                    <i class="fa fa-pencil-square-o" aria-hidden="true"></i>
-                    <span class="caret"></span> <span class="sr-only">Toggle 
Dropdown</span>
-                </button>
-                <ul class="dropdown-menu dropdown-menu">
-                    <li>
-                        <a id="btn-indent-code" href="#">
-                            <span> {{ _('Indent Selection (Tab)') }} </span>
-                        </a>
-                        <a id="btn-unindent-code" href="#">
-                            <span> {{ _('Unindent Selection (Shift+Tab)') }} 
</span>
-                        </a>
-                        <a id="btn-comment-line" href="#">
-                            <span> {{ _('Inline Comment Selection 
(Ctrl+Shift+,)') }} </span>
-                        </a>
-                        <a id="btn-uncomment-line" href="#">
-                            <span> {{ _('Inline Uncomment Selection 
(Ctrl+Shift+.)') }} </span>
-                        </a>
-                        <a id="btn-toggle-comment-block" href="#">
-                            <span> {{ _('Block Comment/Uncomment Selection 
(Ctrl+Shift+/)') }} </span>
-                        </a>
-                    </li>
-                </ul>
-            </div>
-            <div class="btn-group" role="group" aria-label="">
                 <button id="btn-filter" type="button" class="btn btn-default"
                         title="{{ _('Filter') }}" disabled accesskey="i">
                     <i class="fa fa-filter" aria-hidden="true"></i>
@@ -241,6 +214,33 @@
             <div class="btn-group" role="group" aria-label="">
                 <button id="btn-edit-dropdown" type="button" class="btn 
btn-default dropdown-toggle"
                         data-toggle="dropdown" aria-haspopup="true" 
aria-expanded="false"
+                        title="{{ _('Edit') }}" accesskey="e">
+                    <i class="fa fa-pencil-square-o" aria-hidden="true"></i>
+                    <span class="caret"></span> <span class="sr-only">Toggle 
Dropdown</span>
+                </button>
+                <ul class="dropdown-menu dropdown-menu">
+                    <li>
+                        <a id="btn-indent-code" href="#">
+                            <span> {{ _('Indent Selection (Tab)') }} </span>
+                        </a>
+                        <a id="btn-unindent-code" href="#">
+                            <span> {{ _('Unindent Selection (Shift+Tab)') }} 
</span>
+                        </a>
+                        <a id="btn-comment-line" href="#">
+                            <span> {{ _('Inline Comment Selection 
(Ctrl+Shift+,)') }} </span>
+                        </a>
+                        <a id="btn-uncomment-line" href="#">
+                            <span> {{ _('Inline Uncomment Selection 
(Ctrl+Shift+.)') }} </span>
+                        </a>
+                        <a id="btn-toggle-comment-block" href="#">
+                            <span> {{ _('Block Comment/Uncomment Selection 
(Ctrl+Shift+/)') }} </span>
+                        </a>
+                    </li>
+                </ul>
+            </div>
+            <div class="btn-group" role="group" aria-label="">
+                <button id="btn-clear-dropdown" type="button" class="btn 
btn-default dropdown-toggle"
+                        data-toggle="dropdown" aria-haspopup="true" 
aria-expanded="false"
                         title="{{ _('Clear') }}" accesskey="l">
                     <i class="fa fa-eraser" aria-hidden="true"></i>
                     <span class="caret"></span> <span class="sr-only">Toggle 
Dropdown</span>

Reply via email to