Re: Discussion on improving alertify notifications logic

[Akshay Joshi]

Hi Sara

On Fri, Jul 28, 2017 at 9:19 AM, Sarah McAlear  wrote:

> Hi Akshay!
>
> That seems like a good idea. When we're using the extended
> pgadmin.alerrtify in the codebase it would be beneficial to call it
> pgAdminAlertify or something similar so that it is clear that it isn't only
> the alertify library, but it's been extended.
>

   Thanks, but If I understand to you correctly calling it pgAdminAlertify
will required code changes wherever alertify. is called.

>
> Rob & Sarah
>
> On Thu, Jul 27, 2017 at 9:41 PM, Akshay Joshi <
> akshay.jo...@enterprisedb.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.
>>
>> --
>> *Akshay Joshi*
>> *Principal Software Engineer *
>>
>>
>>
>> *Phone: +91 20-3058-9517 <+91%2020%203058%209517>Mobile: +91 976-788-8246
>> <+91%2097678%2088246>*
>>
>
>


-- 
*Akshay Joshi*
*Principal Software Engineer *



*Phone: +91 20-3058-9517Mobile: +91 976-788-8246*

Re: Discussion on improving alertify notifications logic

[Sarah McAlear]

Hi Akshay!

That seems like a good idea. When we're using the extended
pgadmin.alerrtify in the codebase it would be beneficial to call it
pgAdminAlertify or something similar so that it is clear that it isn't only
the alertify library, but it's been extended.

Rob & Sarah

On Thu, Jul 27, 2017 at 9:41 PM, Akshay Joshi  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.
>
> --
> *Akshay Joshi*
> *Principal Software Engineer *
>
>
>
> *Phone: +91 20-3058-9517 <+91%2020%203058%209517>Mobile: +91 976-788-8246
> <+91%2097678%2088246>*
>