Use the displayName property in default editor #73311

timroes · 2020-07-27T16:30:38Z

Summary

This will make the default vis editor to use the displayName instead of the name property of an index pattern field properly. (It currently already showed it after selection, but not in the list.

Easiest way to test this is to enable the shortDots:enable setting under advanced settings. Now all field names containing at least a . should be shortened before that to just one letter per block.

This is a preparation for #69908 where users can specify the displayName themselves.

Known bugs

When opening the checkbox and there are duplicate items the checkmark is shown before both items, not just the one actually selected. That's a bug in EUI and will be still be fixed via (elastic/eui#4162) so ignore this for review, it will be fixed with one of the next EUI upgrades (which happens before the 7.11 release).

Checklist

Delete any items that are not applicable to this PR.

~~[ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support~~
~~[ ] Documentation was added for features that require explanation or tutorials~~
Unit or functional tests were updated or added to match the most common scenarios
~~[ ] This was checked for keyboard-only and screenreader accessibility~~
~~[ ] This renders correctly on smaller devices using a responsive layout. (You can test this in your browser~~
~~[ ] This was checked for cross-browser compatibility~~

For maintainers

~~[ ] This was checked for breaking API changes and was labeled appropriately~~

elasticmachine · 2020-07-27T16:30:41Z

Pinging @elastic/kibana-app (Team:KibanaApp)

src/plugins/data/common/index_patterns/fields/field.ts

@@ -57,7 +57,7 @@ export class Field implements IFieldType {
  visualizable?: boolean;
  scripted?: boolean;
  subType?: IFieldSubType;
-  displayName?: string;
+  displayName: string;


src/plugins/vis_default_editor/public/components/controls/field.tsx

@@ -52,7 +52,7 @@ function FieldParamEditor({
 }: FieldParamEditorProps) {
  const [isDirty, setIsDirty] = useState(false);
  const selectedOptions: ComboBoxGroupedOptions<IndexPatternField> = value
-    ? [{ label: value.displayName || value.name, target: value }]
+    ? [{ label: value.displayName, target: value }]


timroes · 2020-07-27T16:56:06Z

Blocked on elastic/eui#3803

kertal · 2020-07-30T07:11:02Z

src/plugins/vis_default_editor/public/components/agg_params_helper.ts

@@ -93,7 +93,7 @@ function getAggParamsToRender({
        }
      }
      fields = filterAggTypeFields(availableFields, agg);
-      indexedFields = groupAndSortBy(fields, 'type', 'name');
+      indexedFields = groupAndSortBy(fields, 'type', 'displayName');


if there were displayName duplicates, you could prevent troubles with the EuiComboBox by appending the name in this case. So it could be ${displayName} (${name}).

Not sure if we really want this. I mean e.g. the shortDots setting will change the display name. If a user actually will use this, I assume they really want their names shortened, and I have the feeling printing the long name then for them again, kind of defines the purpose of why they used this setting?

I'd just suggest to display it this way, if there are conflicts with duplicates. so users named 2 fields the same way.
or shortDots converts i.have.cookies and i.hate.cookies to i.h.cookies

stratoula

The only "weird" thing that I noticed is the duplicates but as it is going to be fixed on 7.11 then we are fine. I can't find any other regressions and code is LGTM

kertal

Code LGTM, merged with #69908 and worked.

Only thing I noticed, that for some cases it would be useful if the search of the Combobox would also search the keys (the original field names)

timroes · 2020-10-27T11:11:48Z

@elasticmachine merge upstream

kibanamachine · 2020-10-27T12:49:30Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: a081e4f

Metrics [docs]

async chunks size

id	before	after	diff
`visDefaultEditor`	448.8KB	448.8KB	+17.0B

page load bundle size

id	before	after	diff
`visDefaultEditor`	34.4KB	35.4KB	+1.0KB

History

💚 Build #82781 succeeded d4e95d8
💔 Build #82769 failed be292e7
💚 Build #82752 succeeded 89abb0b
💔 Build #64380 failed 813b9af

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

* Use displayName in field list in visualize editor * Set key in combo box * Fix jest tests Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

* master: [Security Solution][Endpoint][Admin] Malware Protections Notify User Version (elastic#81415) Revert "[Actions] Adding `hasAuth` to Webhook Configuration to avoid confusing UX (elastic#81390)" [Maps] Use default format when proxying EMS-files (elastic#79760) [Discover] Deangularize context.html (elastic#81442) Use the displayName property in default editor (elastic#73311) adds bug label to Bug report for Security Solution template (elastic#81643) [ML] Transforms: Remove index field limitation for custom query. (elastic#81467) [Actions] Adding `hasAuth` to Webhook Configuration to avoid confusing UX (elastic#81390) [Task Manager] Mark task as failed if maxAttempts has been met. (elastic#80681) [Lens] Fix URL query loss on redirect (elastic#81475) Log reason for 404 in field existence check (elastic#81315)

* Use displayName in field list in visualize editor * Set key in combo box * Fix jest tests Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

* master: (87 commits) [Actions] Adding `hasAuth` to Webhook Configuration to avoid confusing UX (elastic#81778) [i18n] add get_kibana_translation_paths tests (elastic#81624) [UX] Fix search term reset from url (elastic#81654) [Lens] Improved range formatter (elastic#80132) [Resolver] `SideEffectContext` changes, remove `@testing-library` uses (elastic#81077) [Time to Visualize] Update Library Text with Call to Action (elastic#81667) [docs] Resolving failed Kibana upgrade migrations (elastic#80999) [ftr/menuToggle] provide helper for enhanced menu toggle handling (elastic#81709) [Task Manager] adds basic observability into Task Manager's runtime operations (elastic#77868) Doc changes for stack management and grouped feature privileges (elastic#80486) Added functional test for alerts list filters by status, alert type and action type. Did a code refactoring and splitting for alerts tests. (elastic#81422) [Security Solution][Endpoint][Admin] Malware Protections Notify User Version (elastic#81415) Revert "[Actions] Adding `hasAuth` to Webhook Configuration to avoid confusing UX (elastic#81390)" [Maps] Use default format when proxying EMS-files (elastic#79760) [Discover] Deangularize context.html (elastic#81442) Use the displayName property in default editor (elastic#73311) adds bug label to Bug report for Security Solution template (elastic#81643) [ML] Transforms: Remove index field limitation for custom query. (elastic#81467) [Actions] Adding `hasAuth` to Webhook Configuration to avoid confusing UX (elastic#81390) [Task Manager] Mark task as failed if maxAttempts has been met. (elastic#80681) ...

timroes added Team:Visualizations Visualization editors, elastic-charts and infrastructure Feature:Vis Editor Visualization editor issues v8.0.0 v7.10.0 labels Jul 27, 2020

timroes requested review from stratoula, sulemanof and a team July 27, 2020 16:30

timroes requested a review from a team as a code owner July 27, 2020 16:30

timroes added the release_note:skip Skip the PR/issue when compiling release notes label Jul 27, 2020

timroes commented Jul 27, 2020

View reviewed changes

src/plugins/data/common/index_patterns/fields/field.ts Outdated

@@ -57,7 +57,7 @@ export class Field implements IFieldType {

visualizable?: boolean;

scripted?: boolean;

subType?: IFieldSubType;

displayName?: string;

displayName: string;

This comment was marked as outdated.

Sign in to view

timroes commented Jul 27, 2020

View reviewed changes

timroes requested a review from kertal July 27, 2020 16:32

timroes marked this pull request as draft July 27, 2020 16:51

timroes removed request for kertal, stratoula and sulemanof July 27, 2020 16:51

timroes added the blocked label Jul 27, 2020

kertal reviewed Jul 30, 2020

View reviewed changes

kertal mentioned this pull request Sep 9, 2020

[Discover] Allow custom label for fields via index pattern field management #70039

Merged

6 tasks

timroes mentioned this pull request Sep 28, 2020

[Discover] Allow custom labels via index pattern fields #69908

Closed

3 tasks

kertal added v7.11.0 and removed v7.10.0 labels Oct 12, 2020

Use displayName in field list in visualize editor

89abb0b

timroes force-pushed the fix/displayname-in-visualize branch from 813b9af to 89abb0b Compare October 20, 2020 12:53

timroes removed the request for review from a team October 20, 2020 12:54

Tim Roes added 2 commits October 20, 2020 15:36

Set key in combo box

be292e7

Fix jest tests

d4e95d8

timroes requested a review from stratoula October 20, 2020 18:58

timroes requested a review from kertal October 20, 2020 18:58

timroes marked this pull request as ready for review October 20, 2020 18:59

stratoula approved these changes Oct 21, 2020

View reviewed changes

kertal approved these changes Oct 21, 2020

View reviewed changes

Merge branch 'master' into fix/displayname-in-visualize

a081e4f

timroes merged commit b3af2e9 into elastic:master Oct 27, 2020

timroes deleted the fix/displayname-in-visualize branch October 27, 2020 12:55

timroes mentioned this pull request Oct 27, 2020

[7.x] Use the displayName property in default editor (#73311) #81763

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use the displayName property in default editor #73311

Use the displayName property in default editor #73311

timroes commented Jul 27, 2020 •

edited

Loading

elasticmachine commented Jul 27, 2020

This comment was marked as outdated.

This comment was marked as outdated.

timroes commented Jul 27, 2020

kertal Jul 30, 2020

timroes Jul 30, 2020

kertal Aug 4, 2020

stratoula left a comment

kertal left a comment

timroes commented Oct 27, 2020

kibanamachine commented Oct 27, 2020

Use the displayName property in default editor #73311

Use the displayName property in default editor #73311

Conversation

timroes commented Jul 27, 2020 • edited Loading

Summary

Known bugs

Checklist

For maintainers

elasticmachine commented Jul 27, 2020

This comment was marked as outdated.

This comment was marked as outdated.

timroes commented Jul 27, 2020

kertal Jul 30, 2020

Choose a reason for hiding this comment

timroes Jul 30, 2020

Choose a reason for hiding this comment

kertal Aug 4, 2020

Choose a reason for hiding this comment

stratoula left a comment

Choose a reason for hiding this comment

kertal left a comment

Choose a reason for hiding this comment

timroes commented Oct 27, 2020

kibanamachine commented Oct 27, 2020

💚 Build Succeeded

Metrics [docs]

async chunks size

page load bundle size

History

timroes commented Jul 27, 2020 •

edited

Loading