[Logs UI] Fix endless scrolling to load more entries #49535
Conversation
Zacqary
added
bug
|
Pinging @elastic/logs-metrics-ui (Team:logs-metrics-ui) |
Zacqary
added
the
release_note:skip
💚 Build Succeeded |
| @@ -102,6 +102,11 @@ export class ScrollableLogTextStreamView extends React.PureComponent< | |||
| targetId: null, | |||
| items: [], | |||
| }; | |||
| } else if (hasItems && nextItems.length > prevState.items.length) { | |||
There was a problem hiding this comment.
Is looking at the number of items a sufficient criterion? What if the list of items is replaced by a list of the same length?
There was a problem hiding this comment.
The way the app is built right now, this never happens. The only time the list of items is wholly replaced is when the user jumps to a timestamp that requires all logs data to reload, which unmounts this component and replaces it with the loading screen.
However this also seems to work if I change it to (nextItems.length !== prevState.items.length || nextItems[0] !== prevState.items[0]) so that could make this fix more future-proof.
There was a problem hiding this comment.
True, that shouldn't happen in the context of the app. I like your improvement, though 👍 Let's hope we find an opportunity to clean up the whole state and rendering code soon (and move to hooks and react-virtualized 😇) 🤞 I hope #49154 will give us the opportunity.
💚 Build Succeeded |
* [Logs UI] Fix endless scrolling to load more entries * Make scrollable list state update more robust
* [Logs UI] Fix endless scrolling to load more entries * Make scrollable list state update more robust
Summary
Fixes #48952
Corrects a regression that broke scrolling up and down to load more entries in the logs stream.
Checklist
[ ] This was checked for cross-browser compatibility, including a check against IE11[ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support[ ] Documentation was added for features that require explanation or tutorials[ ] Unit or functional tests were updated or added to match the most common scenarios[ ] This was checked for keyboard-only and screenreader accessibilityFor maintainers