From: | Dave Page <dpage(at)pgadmin(dot)org> |
---|---|
To: | Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com> |
Cc: | pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org> |
Subject: | Re: Discussion on improving alertify notifications logic |
Date: | 2017-07-28 08:21:31 |
Message-ID: | CA+OCxowN3RUt-hNwYW2UpcOCKtLHidNZ=J-A+cWPt3+apPFTsw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgadmin-hackers |
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.
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
From | Date | Subject | |
---|---|---|---|
Next Message | Dave Page | 2017-07-28 10:30:49 | Re: [pgAdmin4][Patch]: Fixed RM #2603 - Import/Export File issues |
Previous Message | Surinder Kumar | 2017-07-28 07:12:35 | [pgAdmin4][Patch]: RM_2596 - Query tool not working in Desktop Runtime on Mac OS X |