public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [PATCH widget-toolkit v2] fix #7721: fix zoom interaction on charts with no initial store
@ 2026-08-11  8:55 Dominik Csapak
  2026-08-28  8:00 ` Christian Ebner
  0 siblings, 1 reply; 2+ messages in thread
From: Dominik Csapak @ 2026-08-11  8:55 UTC (permalink / raw)
  To: pbs-devel

In our current ExtJs version (7.0), having animations disabled on charts
breaks the zoom interaction. One can zoom in and out once, but never
again.

We generally keep animations enabled on our proxmoxRRDChart, but only
after the store's first load, to avoid animating from the empty store to
the initial data. This happens in initComponent on the store that was
given during instantiation.

In some cases, however (e.g. the PBS datastore summary panel), the chart
has no initial store and instead gets one via 'setStore'. Here, the
event of the old (non-existent) store was never fired, so animations
were never activated and the zoom interaction was buggy.

Fix this by overriding `setStore` to install an event handler for the
new store that enables the animations. If this runs more than once, it
would just set the animation value to the same again, which is fine.

Also guard the delay method against calls from `setStore(null)` which
can happen when a component is being destroyed.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v1:
* rebase on master
* guard against empty store when the component is destroyed

  thanks @Christian for catching this. As for the error message there:
  When the destroy fails, the navigation aborts and does not clean up
  some left over components, leading to some lingering events on half
  destroyed components...

 src/panel/RRDChart.js | 39 +++++++++++++++++++++++++++------------
 1 file changed, 27 insertions(+), 12 deletions(-)

diff --git a/src/panel/RRDChart.js b/src/panel/RRDChart.js
index 82615ce..7480cbc 100644
--- a/src/panel/RRDChart.js
+++ b/src/panel/RRDChart.js
@@ -160,6 +160,32 @@ Ext.define('Proxmox.widget.RRDChart', {
         },
     },
 
+    // enable animation after the store is loaded
+    delayAnimationStart() {
+        let me = this;
+        if (!me.store) {
+            return;
+        }
+        me.store.onAfter(
+            'load',
+            function () {
+                me.setAnimation({
+                    duration: 200,
+                    easing: 'easeIn',
+                });
+            },
+            this,
+            { single: true },
+        );
+    },
+
+    setStore: function (store) {
+        let me = this;
+        let res = Ext.chart.CartesianChart.prototype.setStore.call(me, store);
+        me.delayAnimationStart();
+        return res;
+    },
+
     width: 770,
     height: 300,
     animation: false,
@@ -340,18 +366,7 @@ Ext.define('Proxmox.widget.RRDChart', {
             );
         });
 
-        // enable animation after the store is loaded
-        me.store.onAfter(
-            'load',
-            function () {
-                me.setAnimation({
-                    duration: 200,
-                    easing: 'easeIn',
-                });
-            },
-            this,
-            { single: true },
-        );
+        me.delayAnimationStart();
 
         me.checkThemeColors();
 
-- 
2.47.3





^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH widget-toolkit v2] fix #7721: fix zoom interaction on charts with no initial store
  2026-08-11  8:55 [PATCH widget-toolkit v2] fix #7721: fix zoom interaction on charts with no initial store Dominik Csapak
@ 2026-08-28  8:00 ` Christian Ebner
  0 siblings, 0 replies; 2+ messages in thread
From: Christian Ebner @ 2026-08-28  8:00 UTC (permalink / raw)
  To: Dominik Csapak, pbs-devel

On 8/11/26 10:57 AM, Dominik Csapak wrote:
> In our current ExtJs version (7.0), having animations disabled on charts
> breaks the zoom interaction. One can zoom in and out once, but never
> again.
> 
> We generally keep animations enabled on our proxmoxRRDChart, but only
> after the store's first load, to avoid animating from the empty store to
> the initial data. This happens in initComponent on the store that was
> given during instantiation.
> 
> In some cases, however (e.g. the PBS datastore summary panel), the chart
> has no initial store and instead gets one via 'setStore'. Here, the
> event of the old (non-existent) store was never fired, so animations
> were never activated and the zoom interaction was buggy.
> 
> Fix this by overriding `setStore` to install an event handler for the
> new store that enables the animations. If this runs more than once, it
> would just set the animation value to the same again, which is fine.
> 
> Also guard the delay method against calls from `setStore(null)` which
> can happen when a component is being destroyed.
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---

Double checked the animations already fixed in v1 for PBS, that the 
issue when switching between datastores is no longer present in v2 and 
that there are no regressions for PVE.

Reviewed-by: Christian Ebner <c.ebner@proxmox.com>
Tested-by: Christian Ebner <c.ebner@proxmox.com>




^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-08-28  8:00 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-11  8:55 [PATCH widget-toolkit v2] fix #7721: fix zoom interaction on charts with no initial store Dominik Csapak
2026-08-28  8:00 ` Christian Ebner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal