On Thu, Jul 20, 2017 at 10:29 PM, Dave Page <dp...@pgadmin.org> wrote:
> Hi > > On Thu, Jul 20, 2017 at 3:33 PM, Murtuza Zabuawala <murtuza.zabuawala@ > enterprisedb.com> wrote: > >> 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. >> > > Hmm, I moved it there intentionally as it's a more traditional position > and thus more discoverable. > > Can we just launch the browser with a wider size, say, 1280? It's on line > 43 of app_starter.py... > > Yes, that will work too. > >> >> -- >> 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 >>>> >>> >>> >> > > > -- > Dave Page > Blog: http://pgsnake.blogspot.com > Twitter: @pgsnake > > EnterpriseDB UK: http://www.enterprisedb.com > The Enterprise PostgreSQL Company >
