Trigger update when container resizes #28
Conversation
| if (containerRef && containerRef.current) { | ||
| window.addEventListener('resize', callbackResize); | ||
| observer = new ResizeObserver((event) => { | ||
| setEmptyState({}); |
There was a problem hiding this comment.
I tried calling callbackResize here put it interfered with scrolling behavior (resize is called even for scrolling).
There was a problem hiding this comment.
[curious-question]: Are we using setEmptyState as a way to rerender the component, could be possible to use setLoading(false) as well? would that work?
There was a problem hiding this comment.
setLoading(false): Same behavior as before the fix
setLoading(true) or setResizing(true): the log container is empty
| if (!(containerRef && containerRef.current)) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
To prevent manipulation of DOM when the timer callback happens after the container was closed.
|
This seems related to #55 |
| if (containerRef && containerRef.current) { | ||
| window.addEventListener('resize', callbackResize); | ||
| observer = new ResizeObserver((event) => { |
There was a problem hiding this comment.
Could you check the event type here @mmuzikar? if event.type === 'scroll' then we don't do the callbackResize, otherwise we could use callbackResize.
There was a problem hiding this comment.
Do you mean the event in the ResizeObserver? Because there is no type there. Or is this comment meant for the line above?
There was a problem hiding this comment.
Yes, I mean the event here, sorry I thought this is an HTML DOM event here.
There was a problem hiding this comment.
@DaoDaoNoCode I took another look at this and realized Drawer has an onResize callback. I wonder if somehow you could come at it from that angle instead, and somehow trigger the LogViewer to recalculate its layout imperatively when the drawer is resized. It would of course be nice to have a general solution where the LogViewer handles this itself, but perhaps that would be less hacky? I'm not sure how you would trigger it from outside the LogViewer, just a thought.
There was a problem hiding this comment.
@DaoDaoNoCode I took another look at this and realized
Drawerhas anonResizecallback. I wonder if somehow you could come at it from that angle instead, and somehow trigger the LogViewer to recalculate its layout imperatively when the drawer is resized. It would of course be nice to have a general solution where the LogViewer handles this itself, but perhaps that would be less hacky? I'm not sure how you would trigger it from outside the LogViewer, just a thought.
@mturley I tried using onResize callback on drawer in this PR: opendatahub-io/odh-dashboard#2184
Discussion : opendatahub-io/odh-dashboard#2184 (comment)
But it didn't work as expected. onResize it was not recalcualting the lineHeight. Line height recalculation was happening when the window resizes.
There was a problem hiding this comment.
I updated my PR with the changes you requested, I am not sure I'm following the conversation as I am no React expert, if you have a better idea on how to fix this issue, I can close this and let you do it the right way :)
The containerRef didn't trigger any update on resize, which caused the log viewer to update only on scroll:
Peek 2023-05-09 09-55.webm
Now the component updates when the container resizes:
