From: | Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com> |
---|---|
To: | pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org> |
Subject: | Re: Discussion on improving alertify notifications logic |
Date: | 2017-07-31 09:24:42 |
Message-ID: | CANxoLDcSAytvo-y=3wgPh5GavVqXzXt_e9ymedNZbWX6zh2hKA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgadmin-hackers |
Hi All
On Fri, Jul 28, 2017 at 1:51 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>
>
> On Thu, Jul 27, 2017 at 2:41 PM, Akshay Joshi <
> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>
>> Hi All
>>
>> As in commit "Update alertify alerts to use the styling defined in
>> the style guide":
>>
>> https://git.postgresql.org/gitweb/?p=pgadmin4.git;a=commitdiff
>> ;h=2a30a86e7d5e562040500f448fbb0d143ff2cff9
>>
>> https://git.postgresql.org/gitweb/?p=pgadmin4.git;a=commitdiff
>> ;h=f2d2075d81718ec02550fb592851aa330d327b24
>>
>> We have introduce new wrapper class "AlertifyWrapper" and replace calls
>> to alertify.success and alertify.error with following two lines in most
>> of the files
>>
>> var alertifyWrapper = new AlertifyWrapper();
>>
>> alertifyWrapper.success(message); or alertifyWrapper.error(message);
>>
>> For each call we are creating dynamic object of AlertifyWrapper and call
>> the appropriate function. For example there are 20 such calls in a single
>> js file every time are are creating object and call appropriate
>> function.
>>
>> I have tried to improve the logic here and implemented it as below:
>>
>> - Extend alertify and move success, error and info functions from "
>> alertify_wrapper.js" file to "alertify.pgadmin.defaults.js", there
>> will be no use of "alertify_wrapper.js"
>> - Modify only "server.js" as POC, remove 'alertify' and replace
>> 'sources/alerts/alertify_wrapper' with 'pgadmin.alertifyjs' which is
>> nothing but mapping of "alertify.pgadmin.defaults.js" from defines
>> and named the reference object to 'alertify' so no need to change any
>> function call like "alertify.success, alertify.error".
>>
>> One more benefit of the above approach is if in future we want to use the
>> same style for alertify.warning, alertify.info, alertify.message etc..,
>> we will just have to extend that method in "alertify.pgadmin.defaults.js"
>> and no need to change the rest of the function call with AlertifyWrapper.
>>
>>
>> Attached is the POC patch, if it looks good then I'll start working on
>> replacing AlertifyWrapper with the above mentioned approach.
>>
>
> I like the approach - it's definitely cleaner, and saves instantiating a
> new object every time.
>
I have modified the logic to improve the usage of alrtify notification.
Attached is the patch file which contains following:
- Replace 'alertify' with 'pgadmin.alertifyjs' in define[].
- Remove 'sources/alerts/alertify_wrapper' from define[].
- Replace calls var alertifyWrapper = new AlertifyWrapper();
alertifyWrapper.success(message); or
alertifyWrapper*.error(message); *with
appropriate (alertify.success/alertify.error..)
- Modified test case written for alertify wrapper.
Please review it and if it looks good then I'll commit the code.
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
--
*Akshay Joshi*
*Principal Software Engineer *
*Phone: +91 20-3058-9517Mobile: +91 976-788-8246*
Attachment | Content-Type | Size |
---|---|---|
Improved_Alertify_Notification.patch | application/octet-stream | 73.9 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Ashesh Vashi | 2017-07-31 09:28:23 | pgAgent commit: Added support two digits version for PostgreSQL/EDB Pos |
Previous Message | Murtuza Zabuawala | 2017-07-31 07:27:55 | [pgAdmin4][Patch]: Fix the Domain Create dialog issue |