-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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] Hide multi fields in Doc Viewer #101929
[Discover] Hide multi fields in Doc Viewer #101929
Conversation
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.
QQ: I don't think we should keep out multi fields out of sidebar? since then users could no longer quickly create filters for .keyword
fields, could they?
Good point actually. |
Pinging @elastic/kibana-app (Team:KibanaApp) |
[SHOW_MULTIFIELDS]: { | ||
name: i18n.translate('discover.advancedSettings.discover.showMultifields', { | ||
defaultMessage: 'Show multi fields', | ||
}), | ||
description: i18n.translate('discover.advancedSettings.discover.showMultifieldsDescription', { | ||
defaultMessage: `When enabled will show multi fields in document viewer. In majority of cases, multi-fields are the same as regular fields. This option is only available when searchFieldsFromSource is off. `, | ||
}), |
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.
FYI dear @gchaps a new config flag and wording
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.
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.
Here is a suggestion
Show multi-fields
Controls whether multi-fields display in the document table. Available when searchFieldsFromSource
is off.
Notes:
- multi-fields has a hyphen
- Not sure what the document viewer is. Does it refer to the document table?
- Is the second sentence needed? Not sure what "regular field" means.
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.
Document viewer is the part in the expanded view of the document table that shows all fields in the document:
Please note that multifield values will always show up in the document table. This PR introduces changes in the doc viewer specifically.
As for the second sentence, I am open to suggestions on how to improve it. I would like to keep the emphasis on the fact that in 99% of cases, there should be no difference between an original field value and its multi-field counterpart.
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.
Show multi-fields
Controls whether multi-fields display in the expanded document view. In most cases, multi-fields are the same as the original field. This option is only available when searchFieldsFromSource
is off.
@@ -26,6 +28,7 @@ export function DocViewTable({ | |||
const [fieldRowOpen, setFieldRowOpen] = useState({} as Record<string, boolean>); | |||
const [multiFields, setMultiFields] = useState({} as Record<string, string[]>); | |||
const [fieldsWithParents, setFieldsWithParents] = useState([] as string[]); | |||
const showMultiFields = getServices().uiSettings.get(SHOW_MULTIFIELDS); |
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.
quick first thought while reading the code, since the value of SHOW_MULTIFIELDS
is read and used in table.tsx
, I wonder if it is necessary to add this field to html templates and Angular code?
@elasticmachine merge upstream |
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.
telemetry changes LGTM (discover:showMultiFields
addition to ui settings schema)
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, tested locally in Chrome, Firefox, Safari, works as expected. 2 more cleanups and some text tweaking to go, then it should be fine. Thx for taking care of this 👍
src/plugins/discover/public/application/angular/doc_table/doc_table.ts
Outdated
Show resolved
Hide resolved
src/plugins/discover/public/application/angular/doc_table/doc_table.ts
Outdated
Show resolved
Hide resolved
jenkins, test this (restarting due to jenkins upgrade) |
* Add multifields flag in the Advanced settings * Implement hiding of multifields * Update failing unit test * Fix telemetry * Show muti fields in a single doc view * Mock getServices so that tests pass * Readd fields to sidebar * Removing showMultifields flag from angular * Remove unnecessary import * Applying text changes according to Gails suggestion * Fix i18n error Co-authored-by: Kibana Machine <[email protected]>
* Add multifields flag in the Advanced settings * Implement hiding of multifields * Update failing unit test * Fix telemetry * Show muti fields in a single doc view * Mock getServices so that tests pass * Readd fields to sidebar * Removing showMultifields flag from angular * Remove unnecessary import * Applying text changes according to Gails suggestion * Fix i18n error Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Kibana Machine <[email protected]>
💔 Build Failed
Failed CI StepsTest FailuresKibana Pipeline / general / X-Pack API Integration Tests.x-pack/test/api_integration/apis/ml/jobs/close_jobs·ts.apis Machine Learning jobs close_jobs close jobs fail because they are running as ML PoweruserStandard Out
Stack Trace
Kibana Pipeline / general / X-Pack API Integration Tests.x-pack/test/api_integration/apis/ml/jobs/close_jobs·ts.apis Machine Learning jobs close_jobs close jobs fail because they are running as ML PoweruserStandard Out
Stack Trace
Metrics [docs]Public APIs missing comments
Async chunks
Page load bundle
Unknown metric groupsAPI count
async chunk count
History
To update your PR or re-run it, just comment with: |
Summary
Addresses: #96029
This PR adds a switch in the UI settings (
discover:showMultiFields)
, determining whether to show or hide multifields in Discover. The default value isfalse
, as we assume the majority of users will not need them to be visible by default. Note: this setting will toggle multifields from the doc viewer. They will still be visible in the doc table (and datagrid), the sidebar, as well as in the JSON view.Doc viewer without multifields:
Doc viewer with multifields:
Doc table always:
We should add some more documentation around it still.
Checklist