-
Notifications
You must be signed in to change notification settings - Fork 8k
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
[Dataset quality] Fix flyout error when closed after navigation #182026
[Dataset quality] Fix flyout error when closed after navigation #182026
Conversation
…r precedence issues.
Pinging @elastic/obs-ux-logs-team (Team:obs-ux-logs) |
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
/ci |
Works great, thank you |
setFlyoutState, | ||
] = useState(flyoutState); | ||
|
||
useEffect(() => { |
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.
I am not getting why do we need this useEffect, I have a feeling that its not needed
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.
Yes, I've simplified the code and removed useEffect
.
datasetIntegrationsLoading, | ||
}; | ||
}, [dataStreamDetailsLoading, dataStreamSettingsLoading, datasetIntegrationsLoading]); | ||
const loadingState = useSelector(service, (state) => ({ |
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.
Why do we need to keep all these ones together?
I think it's better if they are separate because they are different requests, so we can show information as soon as it arrives instead of waiting for all of them to return data
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.
They are still separate, just the code is simplified.
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.
LGTM! Thank you for applying the changes
💚 Build Succeeded
Metrics [docs]Async chunks
Canvas Sharable Runtime
History
To update your PR or re-run it, just comment with: |
…tic#182026) Fixes elastic#179880 ## Summary The PR fixes the error reported in the parent issue by making sure Dataset Quality flyout more receptive to `undefined` flyout state.
Fixes #179880
Summary
The PR fixes the error reported in the parent issue by making sure Dataset Quality flyout more receptive to
undefined
flyout state.