Hi On Thu, Jul 20, 2017 at 3:33 PM, Murtuza Zabuawala < murtuza.zabuaw...@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... > > > -- > Regards, > Murtuza Zabuawala > EnterpriseDB: http://www.enterprisedb.com > The Enterprise PostgreSQL Company > > On Thu, Jul 20, 2017 at 6:04 PM, Murtuza Zabuawala <murtuza.zabuawala@ > 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
