-
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
[Discover][ES|QL] Reset selected fields when modifying the ES|QL query #185997
[Discover][ES|QL] Reset selected fields when modifying the ES|QL query #185997
Conversation
/ci |
expect(await dataGrid.getHeaderFields()).to.eql(['@timestamp', 'Document']); | ||
}); | ||
}); | ||
} |
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.
@stratoula I made it closer to what we had in 8.13 and would be great if you could verify the use cases which are mentioned in these tests. Are they working as expected now or more modifications are 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.
Thanx Jul, I will check first thing in the morning
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 they are correct 👍 I tested it also locally, everything works as expected
/ci |
/ci |
Pinging @elastic/kibana-data-discovery (Team:DataDiscovery) |
src/plugins/discover/public/application/main/hooks/use_esql_mode.test.tsx
Outdated
Show resolved
Hide resolved
src/plugins/discover/public/application/main/hooks/use_esql_mode.test.tsx
Outdated
Show resolved
Hide resolved
src/plugins/discover/public/application/main/hooks/use_esql_mode.ts
Outdated
Show resolved
Hide resolved
…de.test.tsx Co-authored-by: Matthias Wilhelm <[email protected]>
…de.test.tsx Co-authored-by: Matthias Wilhelm <[email protected]>
src/plugins/discover/public/application/main/hooks/use_esql_mode.ts
Outdated
Show resolved
Hide resolved
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.
Code LGTM, thx for fixing it 👍 . Tested locally an work as expected,
💚 Build Succeeded
Metrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: cc @jughosta |
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 fantastic! LGTM!
Summary
This PR updates the logic of resetting columns when modifying the ES|QL query. The previous implementation was added in #167492 and then changed in #177241.
Checklist