public inbox for [email protected]  
help / color / mirror / Atom feed
From: Aditya Toshniwal <[email protected]>
To: Dave Page <[email protected]>
Cc: Akshay Joshi <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Subject: Re: [pgAdmin4][RM3294] User need to reset the layout to see the changed preferences parameters
Date: Fri, 29 Jun 2018 22:39:23 +0530
Message-ID: <CAM9w-_kYgd1RWCWtVV0uvJa3T_As1Rfj4icp3sOa_4bTKa8ftQ@mail.gmail.com> (raw)
In-Reply-To: <CA+OCxowKQaG4e5H8cfyj8_uKDh-T_cfmb_byKO1RTYB2T0P8tw@mail.gmail.com>
References: <CAM9w-_nZikTmx0JkrDyev3Z8qk26MskoEF2=N1T2-Ji-mK8Q-Q@mail.gmail.com>
	<CANxoLDfCm1Y=Z_8Z4uM8FT3eWfvni+kgcQNMfzyRdJfLcQTb6Q@mail.gmail.com>
	<CAM9w-_kDJSvVFf84Ery7FsW31hsx+Ch_yNzsMoFQX1kYOs-qbw@mail.gmail.com>
	<CAM9w-_=wmMOF7dsMGh6rTQJModYFcd18Z8UuGi-qXEhDedZsBw@mail.gmail.com>
	<CA+OCxowKQaG4e5H8cfyj8_uKDh-T_cfmb_byKO1RTYB2T0P8tw@mail.gmail.com>

Hi Dave,

On Fri, Jun 29, 2018 at 9:30 PM, Dave Page <[email protected]> wrote:

> Hi
>
> On Fri, Jun 29, 2018 at 3:14 PM, Aditya Toshniwal <aditya.toshniwal@
> enterprisedb.com> wrote:
>
>> Hi Hackers,
>>
>> Attached is the updated patch.
>>
>
> This seems to work for the most part, however I saw what seemed like odd
> behaviour. If I have 2 query tool windows open;
>
> - Changing a preference from the Preferences dialogue updates both query
> tools.
>
> - Changing a preference from a Query Tool updates the Preferences dialogue.
>
> - Changing a preference from a Query Tool does *not* update the other
> Query Tool.
>
> The last point seems odd to me, though it also kinda makes sense to not
> have one query tool affect the other. The problem with that is that it
> could get quite confusing when they get out of sync. I think it would be
> better for a change in one Query Tool to update the other(s).
>
> What do you think? Was this behaviour intentional?
>

​No this was not intentional. It should reflect in other query tools as
well because changing the flags like Auto Commit changes the preferences
config and is not local to a Query tool. I missed the fact that some
preferences can be changed from other than preference dialog.​

​Will send the updated patch with the fix.​

>
> (FYI, in case this was a one-off bug, I was testing using "Auto Commit?")
>
>
>>
>> On Fri, Jun 29, 2018 at 4:46 PM, Aditya Toshniwal <
>> [email protected]> wrote:
>>
>>> Hi Akshay,
>>>
>>> On Fri, Jun 29, 2018 at 3:42 PM, Akshay Joshi <
>>> [email protected]> wrote:
>>>
>>>> Hi Aditya
>>>>
>>>> I have applied your patch and run pgAdmin4. I have found following two
>>>> issue in the browser:
>>>>
>>>>    - Found error while open Preferences dialog. Refer
>>>>    Open_Preferences_Dialog.png
>>>>
>>>> ​This error occurs even with the latest pull without changes.​
>>>
>>>
>>>>
>>>>    - Set the preferences setting "Open in new browser tab" to True and
>>>>    open the query tool. Refer "Open_In_New_Broswer.png".
>>>>
>>>> ​Will look into this.​
>>>
>>>
>>>> I haven't review the code.
>>>>
>>>> On Thu, Jun 28, 2018 at 8:04 PM, Aditya Toshniwal <
>>>> [email protected]> wrote:
>>>>
>>>>> Hi Hackers,
>>>>>
>>>>> Attached is the patch for making preferences realtime and applying
>>>>> without reseting the layout. Please note, the patch is only for one module
>>>>> - SQL Editor and is the first part for the RM. There are lot of changes to
>>>>> be done to cover all and hence sending in parts. This will not affect/break
>>>>> existing code. Further patches will cover other modules.
>>>>>
>>>>> Highlights of this patch include:
>>>>> - Changes will affect SQL Editors in Create dialog boxes, SQL tab of
>>>>> the main screen, Query tool, History entries in the query tool, Query tool
>>>>> opened in New Tab/Window
>>>>> - All the components of SQL editor will refer to single source of
>>>>> preferences which is cached in the Browser object. All other redundant ajax
>>>>> get preference calls are removed.
>>>>> - SQL editor will not refer template JS variables anymore, once all
>>>>> the references are removed the template variables will also be removed.
>>>>> - Code refactoring wherever possible.
>>>>> - Covered JS test cases wherever possible.
>>>>>
>>>>> Request you to kindly review.
>>>>>
>>>>> --
>>>>> Thanks and Regards,
>>>>> Aditya Toshniwal
>>>>> Software Engineer | EnterpriseDB Software Solutions | Pune
>>>>> "Don't Complain about Heat, Plant a tree"
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> *Akshay Joshi*
>>>>
>>>> *Sr. Software Architect *
>>>>
>>>>
>>>>
>>>> *Phone: +91 20-3058-9517Mobile: +91 976-788-8246*
>>>>
>>>
>>>
>>>
>>> --
>>> Thanks and Regards,
>>> Aditya Toshniwal
>>> Software Engineer | EnterpriseDB Software Solutions | Pune
>>> "Don't Complain about Heat, Plant a tree"
>>>
>>
>>
>>
>> --
>> Thanks and Regards,
>> Aditya Toshniwal
>> Software Engineer | EnterpriseDB Software Solutions | Pune
>> "Don't Complain about Heat, Plant a tree"
>>
>
>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>



-- 
Thanks and Regards,
Aditya Toshniwal
Software Engineer | EnterpriseDB Software Solutions | Pune
"Don't Complain about Heat, Plant a tree"


view thread (14+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: [pgAdmin4][RM3294] User need to reset the layout to see the changed preferences parameters
  In-Reply-To: <CAM9w-_kYgd1RWCWtVV0uvJa3T_As1Rfj4icp3sOa_4bTKa8ftQ@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox