pierrejeambrun commented on code in PR #27560:
URL: https://github.com/apache/airflow/pull/27560#discussion_r1016969068
##########
airflow/www/static/js/dag/Main.tsx:
##########
@@ -62,6 +64,9 @@ const Main = () => {
onStatusHover.cancel();
};
+ const gridWidth = localStorage.getItem(`grid-width-${dagId}`) || undefined;
+ const saveWidth = debounce((w) =>
localStorage.setItem(`grid-width-${dagId}`, w));
Review Comment:
Default value for `debounce` is 0 ms (~ no debounce). We should specify a
value:
```suggestion
const saveWidth = debounce((w) =>
localStorage.setItem(`grid-width-${dagId}`, w), localStorageDelay);
```
I tried that locally, but this change is not enough => then the function is
only triggered when we stop resizing. (not during resizing, which could be
great if this was expected :joy: ) Also, maybe those functions should be
defined outside of the component. I don't think they should change between
renders, or should be bound to the component.
##########
airflow/www/static/js/dag/Main.tsx:
##########
@@ -77,9 +82,11 @@ const Main = () => {
const resize = useCallback((e: MouseEvent) => {
const gridEl = gridRef.current;
if (gridEl && e.x > minPanelWidth && e.x < window.innerWidth -
minPanelWidth) {
- gridEl.style.width = `${e.x}px`;
+ const width = `${e.x}px`;
+ gridEl.style.width = width;
+ saveWidth(width);
}
- }, [gridRef]);
+ }, [gridRef, saveWidth]);
Review Comment:
yep savewidth could be defined outside of the component. I think debounce
returns a new function on each render, so this will trigger on each render,
making the calledback useless
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
