-
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
[DataView] show empty matching sources when no matched index pattern #195537
Merged
davismcphee
merged 14 commits into
elastic:main
from
lsq645599166:data_view_index_no_matched
Oct 24, 2024
Merged
Changes from 8 commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
6af9e25
[DataView] show empty matching sources when no matched index pattern
lsq645599166 1d447d2
Merge branch 'main' into data_view_index_no_matched
kertal c6b9cd3
[DataView] redirect to all Sources when no matched index
lsq645599166 1d8a8d5
[DataView] fix unit test
lsq645599166 010d44b
Merge branch 'main' into data_view_index_no_matched
elasticmachine 9da2b78
Merge branch 'main' into data_view_index_no_matched
elasticmachine 46098e3
[DataView] add no matched helper text
lsq645599166 eabcd08
Merge branch 'main' into data_view_index_no_matched
elasticmachine dc193e9
[DataView] hide tab when no matched index
lsq645599166 6f820db
Merge branch 'main' into data_view_index_no_matched
elasticmachine a157db9
Merge branch 'main' into data_view_index_no_matched
davismcphee 7ff4950
[Dataview] revert getMatchedIndices
lsq645599166 0b27ea4
Merge branch 'main' into data_view_index_no_matched
elasticmachine fb3add3
Merge branch 'main' into data_view_index_no_matched
davismcphee File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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
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.
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.
Thanks again for the contribution @lsq645599166, and apologies for all the back and forth on this. After testing this locally, I think misunderstood the intent of the code originally. I also overlooked @kertal's second comment where he suggested hiding the tabs entirely, but I agree this is actually the best approach.
We already do this before the user has typed anything, and we show the full list of sources:
We also intentionally return the full list as
visibleIndices
fromgetMatchedIndices
when there are no matches because we want the user to see the available options when their pattern doesn't match. It turns out this was behaving as intended, but the confusing part was that the tabs were still visible and "Matching sources" was still selected.If we hide the tabs when there are no matches as @kertal suggested, it behaves the same as if the user hadn't typed anything, so there's no confusion about the tabs, and they'll still be able to see the available sources:
This seems like the least confusing and most consistent behaviour (and matches the original intent). Luckily if we follow the same logic used in
src/plugins/data_view_editor/public/components/preview_panel/status_message/status_message.tsx
, I think we should be able to revert all changes except for this line inPreviewPanel
.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.
Ok, I commit a change to hide tab when no matched index.