Re: [pgAdmin4][Patch]: RM #1978 - Add an option to allow user to disable alertifyjs and acitree animations

[Dave Page]

Thanks, applied.

On Wed, Apr 4, 2018 at 6:26 AM, Khushboo Vashi <
khushboo.va...@enterprisedb.com> wrote:

> Hi Dave,
>
> On Tue, Apr 3, 2018 at 7:23 PM, Dave Page  wrote:
>
>> Hi
>>
>> Thanks - I've committed this, however, could you send me an updated
>> screenshot for the docs? The one you sent was a different size and colour
>> depth from the others (and looked like a different scale).
>>
>> Please find the attached patch for the same.
>
>> On Tue, Apr 3, 2018 at 10:42 AM, Khushboo Vashi <
>> khushboo.va...@enterprisedb.com> wrote:
>>
>>> Hi,
>>>
>>> Please find the attached updated patch.
>>>
>>> On Thu, Mar 29, 2018 at 6:54 PM, Dave Page  wrote:
>>>
 Hi

 On Thu, Mar 29, 2018 at 1:51 PM, Khushboo Vashi <
 khushboo.va...@enterprisedb.com> wrote:

>
>
> On Mon, Mar 26, 2018 at 6:07 PM, Dave Page  wrote:
>
>> Hi
>>
>> On Mon, Mar 26, 2018 at 7:23 AM, Khushboo Vashi <
>> khushboo.va...@enterprisedb.com> wrote:
>>
>>> Hi,
>>>
>>> Please find the attached patch to fix RM #1978: Add an option to
>>> allow user to disable alertifyjs and acitree animations.
>>>
>>
>> I think these really need to be per-user settings, not
>> per-installation.. Whether or not animations are shown is really a matter
>> of personal taste and circumstance.
>>
>> Right, it should be per-user settings.  Please find the attached
> updated patch.
>

 I found some issues I'm afraid:

 - The label "Enable dialogues/notifications animation?" should read
 "Enable dialogue/notification animation?"

 Changed.
>>>
 - Disabling treeview animation only seems to affect the main browser
 treeview, and not others in the application (e.g. the one on the
 Preferences panel).

 Fixed
>>>
  - After disabling dialogue/notification animations, I cannot re-enable
 notification animations. If I flip the switch back on, dialogue animations
 immediately start working again, but notification animations don't even
 work following a reload.

 Fixed.
>>>
 Thanks.

 --
 Dave Page
 Blog: http://pgsnake.blogspot.com
 Twitter: @pgsnake

 EnterpriseDB UK: http://www.enterprisedb.com
 The Enterprise PostgreSQL Company

>>>
>>> Thanks,
>>> Khushboo
>>>
>>
>>
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>
> Thanks,
> Khushboo
>



-- 
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: [pgAdmin4][Patch]: RM #1978 - Add an option to allow user to disable alertifyjs and acitree animations

[Dave Page]

Hi

Thanks - I've committed this, however, could you send me an updated
screenshot for the docs? The one you sent was a different size and colour
depth from the others (and looked like a different scale).

On Tue, Apr 3, 2018 at 10:42 AM, Khushboo Vashi <
khushboo.va...@enterprisedb.com> wrote:

> Hi,
>
> Please find the attached updated patch.
>
> On Thu, Mar 29, 2018 at 6:54 PM, Dave Page  wrote:
>
>> Hi
>>
>> On Thu, Mar 29, 2018 at 1:51 PM, Khushboo Vashi <
>> khushboo.va...@enterprisedb.com> wrote:
>>
>>>
>>>
>>> On Mon, Mar 26, 2018 at 6:07 PM, Dave Page  wrote:
>>>
 Hi

 On Mon, Mar 26, 2018 at 7:23 AM, Khushboo Vashi <
 khushboo.va...@enterprisedb.com> wrote:

> Hi,
>
> Please find the attached patch to fix RM #1978: Add an option to allow
> user to disable alertifyjs and acitree animations.
>

 I think these really need to be per-user settings, not
 per-installation.. Whether or not animations are shown is really a matter
 of personal taste and circumstance.

 Right, it should be per-user settings.  Please find the attached
>>> updated patch.
>>>
>>
>> I found some issues I'm afraid:
>>
>> - The label "Enable dialogues/notifications animation?" should read
>> "Enable dialogue/notification animation?"
>>
>> Changed.
>
>> - Disabling treeview animation only seems to affect the main browser
>> treeview, and not others in the application (e.g. the one on the
>> Preferences panel).
>>
>> Fixed
>
>>  - After disabling dialogue/notification animations, I cannot re-enable
>> notification animations. If I flip the switch back on, dialogue animations
>> immediately start working again, but notification animations don't even
>> work following a reload.
>>
>> Fixed.
>
>> Thanks.
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>
> Thanks,
> Khushboo
>



-- 
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Re: [pgAdmin4][Patch]: RM #1978 - Add an option to allow user to disable alertifyjs and acitree animations

[Khushboo Vashi]

Hi Joao,

Thanks for the review. I have sent the patch.

On Thu, Mar 29, 2018 at 8:46 PM, Joao De Almeida Pereira <
jdealmeidapere...@pivotal.io> wrote:

> Hi Khushboo,
>
> The patch looks like it is heading on the correct direction, we passed it
> through our test pipeline and all tests passed.
>
> We found the same issues as Dave mentioned in his email, also after some
> code review we have the following questions/comments:
>
> Fixed

> . Why does modify_animation.js have a dependency on sources/pgadmin if it
> doesnt use it?
>
Removed, it was by mistake

> . Can we convert modify_animation.js to ES6 without requirejs?
>
Done

> . Why does modifyAnimation function have 2 arguments if we never use the
> second one?
>
Not applicable now as it has been changed.

> . Can we convert modify_animation_spec.js to ES6 without requirejs?
>
Done

> . Why is pgBrowser.tree.options function called 4 times in the tests?
>
While setting tree options, it has been used 4 times.

>As an aside, when you use toHaveBeenCalledWith it is redundant to
> expect on toHaveBeenCalled
>
Thanks for the information

> . Looks like we are missing some coverage of the alertify modification as
> well
>
I have improved the coverage.

>
>

> As an aside get_preferences, the "cache", is still broken, if the cache as
> no value it will retrieve it but returns undefined to the caller. This
> behavior need to be addressed. We should change get preferences to be a
> Promise based thing or else this might become a problem
>
> Will check and fix.

> As another aside, one of our goals should be to move away from requirejs
> into a full ES6, webpack javascript build. In order to do that we should
> try to write the least amount of code possible using requirejs syntax. If
> we really need to write something in requirejs let it be a wrapper that
> call our ES6 function/class
>
> Thanks
> Joao
>
> Thanks,
Khushboo

> On Thu, Mar 29, 2018 at 9:25 AM Dave Page  wrote:
>
>> Hi
>>
>> On Thu, Mar 29, 2018 at 1:51 PM, Khushboo Vashi <
>> khushboo.va...@enterprisedb.com> wrote:
>>
>>>
>>>
>>> On Mon, Mar 26, 2018 at 6:07 PM, Dave Page  wrote:
>>>
 Hi

 On Mon, Mar 26, 2018 at 7:23 AM, Khushboo Vashi <
 khushboo.va...@enterprisedb.com> wrote:

> Hi,
>
> Please find the attached patch to fix RM #1978: Add an option to allow
> user to disable alertifyjs and acitree animations.
>

 I think these really need to be per-user settings, not
 per-installation.. Whether or not animations are shown is really a matter
 of personal taste and circumstance.

 Right, it should be per-user settings.  Please find the attached
>>> updated patch.
>>>
>>
>> I found some issues I'm afraid:
>>
>> - The label "Enable dialogues/notifications animation?" should read
>> "Enable dialogue/notification animation?"
>>
>> - Disabling treeview animation only seems to affect the main browser
>> treeview, and not others in the application (e.g. the one on the
>> Preferences panel).
>>
>>  - After disabling dialogue/notification animations, I cannot re-enable
>> notification animations. If I flip the switch back on, dialogue animations
>> immediately start working again, but notification animations don't even
>> work following a reload.
>>
>> Thanks.
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>

Re: [pgAdmin4][Patch]: RM #1978 - Add an option to allow user to disable alertifyjs and acitree animations

[Khushboo Vashi]

Hi Joao,

On Tue, Mar 27, 2018 at 12:12 AM, Joao De Almeida Pereira <
jdealmeidapere...@pivotal.io> wrote:

> Hi Khushboo,
>
> Looks like you have a typo on your CSS where it reads 'zoomeIn' it should
> be 'zoomIn'.
>
Thanks, I have sent the updated patch.

> Also setting more parameters into the window, in our experience, is never
> good, so maybe it is time to create a real settings cache that can retrieve
> from the backend the settings, like this one.
>
> We already have a cache for preferences, so I have used that in the
updated patch.

> Thanks
> Victoria & Joao
>
> Thanks,
Khushboo

> On Mon, Mar 26, 2018 at 8:38 AM Dave Page  wrote:
>
>> Hi
>>
>> On Mon, Mar 26, 2018 at 7:23 AM, Khushboo Vashi <
>> khushboo.va...@enterprisedb.com> wrote:
>>
>>> Hi,
>>>
>>> Please find the attached patch to fix RM #1978: Add an option to allow
>>> user to disable alertifyjs and acitree animations.
>>>
>>
>> I think these really need to be per-user settings, not per-installation..
>> Whether or not animations are shown is really a matter of personal taste
>> and circumstance.
>>
>> Thanks.
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>

Re: [pgAdmin4][Patch]: RM #1978 - Add an option to allow user to disable alertifyjs and acitree animations

[Joao De Almeida Pereira]

Hi Khushboo,

Looks like you have a typo on your CSS where it reads 'zoomeIn' it should
be 'zoomIn'.
Also setting more parameters into the window, in our experience, is never
good, so maybe it is time to create a real settings cache that can retrieve
from the backend the settings, like this one.

Thanks
Victoria & Joao

On Mon, Mar 26, 2018 at 8:38 AM Dave Page  wrote:

> Hi
>
> On Mon, Mar 26, 2018 at 7:23 AM, Khushboo Vashi <
> khushboo.va...@enterprisedb.com> wrote:
>
>> Hi,
>>
>> Please find the attached patch to fix RM #1978: Add an option to allow
>> user to disable alertifyjs and acitree animations.
>>
>
> I think these really need to be per-user settings, not per-installation..
> Whether or not animations are shown is really a matter of personal taste
> and circumstance.
>
> Thanks.
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

[pgAdmin4][Patch]: RM #1978 - Add an option to allow user to disable alertifyjs and acitree animations

[Khushboo Vashi]

Hi,

Please find the attached patch to fix RM #1978: Add an option to allow user
to disable alertifyjs and acitree animations.

Thanks,
Khushboo
diff --git a/web/config.py b/web/config.py
index 3d8f6f7..a91b2e2 100644
--- a/web/config.py
+++ b/web/config.py
@@ -364,6 +364,12 @@ COOKIE_DEFAULT_DOMAIN = None
 SESSION_COOKIE_DOMAIN = None
 
 ##
+# Allow user to disable alertify and acitree animation
+##
+SHOW_ALERTIFY_ANIMATION = True
+SHOW_ACITREE_ANIMATION = True
+
+##
 # Local config settings
 ##
 
diff --git a/web/pgadmin/browser/static/js/browser.js b/web/pgadmin/browser/static/js/browser.js
index 46b6033..31e6db9 100644
--- a/web/pgadmin/browser/static/js/browser.js
+++ b/web/pgadmin/browser/static/js/browser.js
@@ -73,14 +73,16 @@ define('pgadmin.browser', [
 },
 loaderDelay: 100,
 show: {
-  duration: 75,
+  duration: window.SHOW_ACITREE_ANIMATION ? 75 : 0,
 },
 hide: {
-  duration: 75,
+  duration: window.SHOW_ACITREE_ANIMATION ? 75 : 0,
 },
 view: {
-  duration: 75,
+  duration: window.SHOW_ACITREE_ANIMATION ? 75 : 0,
 },
+animateRoot: window.SHOW_ACITREE_ANIMATION ? true : false,
+unanimated: window.SHOW_ACITREE_ANIMATION ? false : true,
   });
 
   b.tree = $('#tree').aciTree('api');
diff --git a/web/pgadmin/static/css/alertify.noanimation.css b/web/pgadmin/static/css/alertify.noanimation.css
new file mode 100644
index 000..cc68747
--- /dev/null
+++ b/web/pgadmin/static/css/alertify.noanimation.css
@@ -0,0 +1,41 @@
+.alertify .ajs-dimmer,
+.alertify .ajs-modal,
+.alertify-notifier,
+.alertify-notifier .ajs-message.ajs-visible,
+.alertify-notifier .ajs-message,
+.alertify-notifier.ajs-center .ajs-message.ajs-visible,
+.alertify-notifier.ajs-center .ajs-message
+{
+  -moz-transition: none;
+  -webkit-transition: none;
+  -o-transition: none;
+  transition: none;
+  -webkit-transform: none;
+  transform: none;
+}
+
+.alertify.ajs-zoom.ajs-in:not(.ajs-hidden) .ajs-dialog {
+  -webkit-animation-name: ajs-zoomeIn;
+  animation-name: ajs-zoomeIn;
+}
+.alertify.ajs-zoom.ajs-out.ajs-hidden .ajs-dialog {
+  -webkit-animation-name: ajs-zoomOut;
+  animation-name: ajs-zoomOut;
+}
+
+@-webkit-keyframes ajs-zoomeIn {
+  -webkit-transform: none;
+  transform: none;
+}
+@keyframes ajs-zoomeIn {
+  -webkit-transform: none;
+  transform: none;
+}
+@-webkit-keyframes ajs-zoomOut {
+  -webkit-transform: none;
+  transform: none;
+}
+@keyframes ajs-zoomeOut {
+  -webkit-transform: none;
+  transform: none;
+}
diff --git a/web/pgadmin/templates/base.html b/web/pgadmin/templates/base.html
index c7a6174..e07536e 100755
--- a/web/pgadmin/templates/base.html
+++ b/web/pgadmin/templates/base.html
@@ -23,6 +23,12 @@
 
 
 
+
+var SHOW_ACITREE_ANIMATION = ('{{ config.SHOW_ACITREE_ANIMATION }}' === 'True') ? true : false;
+
+{% if not config.SHOW_ALERTIFY_ANIMATION %}
+
+{% endif%}
 
  
 {% block css_link %}{% endblock %}