fix: task status indicator not updating live in task view sidebar#175
Open
Kunal-Darekar wants to merge 1 commit intoNetflix:masterfrom
Open
fix: task status indicator not updating live in task view sidebar#175Kunal-Darekar wants to merge 1 commit intoNetflix:masterfrom
Kunal-Darekar wants to merge 1 commit intoNetflix:masterfrom
Conversation
When viewing a task, the red/green/light-green status line next to each task name in the sidebar was not updating in real time as tasks finished. You had to reload the page to see the new status. Root cause: react-virtualized's List component caches row renders for performance. When WebSocket updates arrived and task data in props changed, the virtualized list didn't re-render already-visible rows, so the status indicator stayed stale. The previous code had a scroll event listener that called setScrollTop() on every window scroll, which accidentally caused re-renders that flushed the row cache. That was an unintentional workaround, not a real fix. Fix: add a ref on the List component and call forceUpdateGrid() inside a useEffect that runs whenever the rows prop changes. This is the correct react-virtualized API for telling the list to re-render its visible rows with fresh data. The scroll listener is kept because it's still needed for the listSize height calculation (to handle the sticky header transition on scroll). Fixes: Netflix#80
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this fixes
Closes #80 (from upstream Netflix/metaflow-ui)
When viewing a task in the task view, the status indicator (the colored line next to each task name in the left sidebar) was not updating in real time as tasks finished. You had to reload the page to see the updated status, even though the timeline view updated correctly.
Root cause
TaskList.tsx uses react-virtualized's
<List>component to render the sidebar task rows. react-virtualized aggressively caches row renders for performance — when task data updates arrive via WebSocket and therowsprop changes, the component sees new props but react-virtualized does not automatically re-render already-visible rows with the fresh data.The old code had a
window.scrolllistener that calledsetScrollTop()on every scroll event. This worked as an accidental workaround — scrolling forced a re-render, which flushed the row cache and showed updated statuses. But if the user never scrolled, the status would never update.Fix
Added a
refon the<List>component and callforceUpdateGrid()inside auseEffectthat fires wheneverrowschanges: