-
Notifications
You must be signed in to change notification settings - Fork 8.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Stack Monitoring] Elasticsearch Overview view migration #111941
[Stack Monitoring] Elasticsearch Overview view migration #111941
Conversation
Pinging @elastic/logs-metrics-ui (Team:logs-metrics-ui) |
if (onBrushHappenedRef.current) { | ||
setOnBrushHappened(false); | ||
} else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The pop state event is also fired when the onBrush event is fired, and I couldn't find out why this happens. So I added a flag to know when the onBrush event is fired to prevent changing the zoom level at the wrong moment.
In Angular, this was handled by removing the listener before updating the state and adding it again after some milliseconds, but the same trick didn't work in React.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting. Might be good to capture this in a code comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me overall. I'll hook it up to a cloud QA cluster to check logs and additional tabs before approving.
if (onBrushHappenedRef.current) { | ||
setOnBrushHappened(false); | ||
} else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting. Might be good to capture this in a code comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💚 Build SucceededMetrics [docs]Module Count
Async chunks
History
To update your PR or re-run it, just comment with: |
) * Elasticsearch overview first version * Fix timezone in react components * Extraxt on brush behaviour from elasticsearch overview page * Fix overview component type * Add elasticsearch pages template with some tabs * Conditionally add some tabs for elasticsearch pages * fix import and types * Filter angular errors in react * remove disabled property from tabs * Add comment
💚 Backport successful
This backport PR will be merged automatically after passing CI. |
…112233) * Elasticsearch overview first version * Fix timezone in react components * Extraxt on brush behaviour from elasticsearch overview page * Fix overview component type * Add elasticsearch pages template with some tabs * Conditionally add some tabs for elasticsearch pages * fix import and types * Filter angular errors in react * remove disabled property from tabs * Add comment Co-authored-by: Ester Martí Vilaseca <[email protected]>
Summary
This PR is part of #111756
It migrates the Elasticsearch Overview page to react and adds an elasticsearch template that contains the tabs for all the Elaticsearch views.
It also migrates the onBrush event callback so it can be reused on different views.
NOTE: when the URL changes dates (zoom out link / browser go back arrow), these changes are not reflected in the datepicker. I opened a separate bug #112063 to fix this (details in that issue to come)