Re: [pgAdmin4][RM3849] Ask to save unsaved query changes stopped working

From: Aditya Toshniwal <aditya(dot)toshniwal(at)enterprisedb(dot)com>
To: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
Cc: Dave Page <dave(dot)page(at)enterprisedb(dot)com>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgAdmin4][RM3849] Ask to save unsaved query changes stopped working
Date: 2019-01-23 06:02:02
Message-ID: CAM9w-_==_XzpT+JtUQueqaNJK-TJa-XEZy__6mA2z0YnrzLErw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi Hackers,

PFA revised patch as per review.

On Wed, Jan 23, 2019 at 11:20 AM Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
wrote:

> Hi Aditya
>
> Found one issue you have remove the function "*disable_tree_state_save*" from
> "*web/regression/python_test_utils/test_utils.py*" and move that code
> into "*set_preference*" function and it is called from "
> *web/regression/runtests.py*" which is conditional, so please correct
> that behaviour.
>
> On Tue, Jan 22, 2019 at 6:28 PM Aditya Toshniwal <
> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>
>> Hi Hackers,
>>
>> Attached is the revised patch. The warning occurrence is configurable
>> from Preferences > Browser > Display.
>>
>> Kindly review.
>>
>> On Tue, Jan 22, 2019 at 5:38 PM Dave Page <dave(dot)page(at)enterprisedb(dot)com>
>> wrote:
>>
>>> On Tue, Jan 22, 2019 at 12:01 PM Aditya Toshniwal
>>> <aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>> >
>>> >
>>> >
>>> > On Tue, Jan 22, 2019 at 5:27 PM Dave Page <dave(dot)page(at)enterprisedb(dot)com>
>>> wrote:
>>> >>
>>> >> On Tue, Jan 22, 2019 at 11:54 AM Aditya Toshniwal
>>> >> <aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>> >> >
>>> >> > Any suggestions on preferences name/desc ?
>>> >>
>>> >> Name: "Confirm on close"
>>> >> Desc: "Confirm closure of the browser or browser tab is intended
>>> >> before proceeding."
>>> >
>>> > It also warns on refresh.
>>>
>>> Name: "Confirm on close or refresh"
>>> Desc: "Confirm closure or refresh of the browser or browser tab is
>>> intended before proceeding."
>>>
>>> >>
>>> >>
>>> >>
>>> >> > On Tue, Jan 22, 2019 at 3:01 PM Dave Page <
>>> dave(dot)page(at)enterprisedb(dot)com> wrote:
>>> >> >>
>>> >> >> Hi
>>> >> >>
>>> >> >> On Tue, Jan 22, 2019 at 6:27 AM Akshay Joshi
>>> >> >> <akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>> >> >> >
>>> >> >> > Hi Aditya
>>> >> >> >
>>> >> >> > Below are my review comments:
>>> >> >> >
>>> >> >> > You have added new config parameter
>>> "DISABLE_BEFOREUNLOAD_MESSAGE" to make this request configurable, should it
>>> be the part of preferences setting instead of configuration? @Dave can you
>>> please comment on this.
>>> >> >>
>>> >> >> That does seem like something that should be a per-user preference,
>>> >> >> not a global config option.
>>> >> >>
>>> >> >> > Name "DISABLE_BEFOREUNLOAD_MESSAGE" is not clear as users
>>> perspective, can you please change it to some meaningful name.
>>> >> >> >
>>> >> >> > Apart from that code looks good to me.
>>> >> >> >
>>> >> >> > On Mon, Jan 21, 2019 at 1:24 PM Aditya Toshniwal <
>>> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>> >> >> >>
>>> >> >> >> Hi Hackers,
>>> >> >> >>
>>> >> >> >> Attached is the patch to throw alert warning when user reloads
>>> or closes the browser.
>>> >> >> >>
>>> >> >> >> 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-9517
>>> >> >> > Mobile: +91 976-788-8246
>>> >> >>
>>> >> >>
>>> >> >>
>>> >> >> --
>>> >> >> Dave Page
>>> >> >> VP, Chief Architect, Tools & Installers
>>> >> >> EnterpriseDB: http://www.enterprisedb.com
>>> >> >> The Enterprise PostgreSQL Company
>>> >> >>
>>> >> >> Blog: http://pgsnake.blogspot.com
>>> >> >> Twitter: @pgsnake
>>> >> >
>>> >> >
>>> >> >
>>> >> > --
>>> >> > Thanks and Regards,
>>> >> > Aditya Toshniwal
>>> >> > Software Engineer | EnterpriseDB Software Solutions | Pune
>>> >> > "Don't Complain about Heat, Plant a tree"
>>> >>
>>> >>
>>> >>
>>> >> --
>>> >> Dave Page
>>> >> VP, Chief Architect, Tools & Installers
>>> >> EnterpriseDB: http://www.enterprisedb.com
>>> >> The Enterprise PostgreSQL Company
>>> >>
>>> >> Blog: http://pgsnake.blogspot.com
>>> >> Twitter: @pgsnake
>>> >
>>> >
>>> >
>>> > --
>>> > Thanks and Regards,
>>> > Aditya Toshniwal
>>> > Software Engineer | EnterpriseDB Software Solutions | Pune
>>> > "Don't Complain about Heat, Plant a tree"
>>>
>>>
>>>
>>> --
>>> Dave Page
>>> VP, Chief Architect, Tools & Installers
>>> EnterpriseDB: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>> Blog: http://pgsnake.blogspot.com
>>> Twitter: @pgsnake
>>>
>>
>>
>> --
>> 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"

Attachment Content-Type Size
RM3849_v3.patch application/octet-stream 7.0 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Akshay Joshi 2019-01-23 06:27:46 pgAdmin 4 commit: Ensure that browser should warn before close or refre
Previous Message Akshay Joshi 2019-01-23 05:49:55 Re: [pgAdmin4][RM3849] Ask to save unsaved query changes stopped working