[Discover] Add 'Copy value' button to field value cells (#217457)

[AlexGPlay]

Summary

Added a "Copy value" button to the field value cells.

Solves #217457

Checklist

Check the PR satisfies following conditions.

Reviewers should verify this PR satisfies this list as well.

• 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
• If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
• This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The release_note:breaking label should be applied in these situations.
• Flaky Test Runner was used on any tests changed
• The PR description includes the appropriate Release Notes section, and the correct release_note:* label is applied per the guidelines

Identify risks

Does this PR introduce any risks? For example, consider risks like hard to test bugs, performance regression, potential of data loss.

Describe the risk, its severity, and mitigation for each identified risk. Invite stakeholders and evaluate how to proceed before merging.

• See some risk examples
• ...

[AlexGPlay]

i was wondering if this action should have some feedback, maybe something like a toast

[jughosta]

Yes, that would be nice!

For example for the main Discover table it's handled in

kibana/src/platform/packages/shared/kbn-unified-data-table/src/utils/copy_value_to_clipboard.ts

Lines 29 to 69 in bb877cf

	export const copyValueToClipboard = ({
	rowIndex,
	columnId,
	toastNotifications,
	valueToStringConverter,
	}: {
	rowIndex: number;
	columnId: string;
	toastNotifications: ToastsStart;
	valueToStringConverter: ValueToStringConverter;
	}): string | null => {
	const result = valueToStringConverter(rowIndex, columnId);
	const valueFormatted = result.formattedString;
	const copied = copyToClipboard(valueFormatted);
	if (!copied) {
	toastNotifications.addWarning({
	title: COPY_FAILED_ERROR_MESSAGE,
	});
	return null;
	}
	const toastTitle = i18n.translate('unifiedDataTable.copyValueToClipboard.toastTitle', {
	defaultMessage: 'Copied to clipboard',
	});
	if (result.withFormula) {
	toastNotifications.addWarning({
	title: toastTitle,
	text: WARNING_FOR_FORMULAS,
	});
	} else {
	toastNotifications.addInfo({
	title: toastTitle,
	});
	}
	return valueFormatted;
	};

Also, it should be copying the text representation of the value (example:

kibana/src/platform/packages/shared/kbn-unified-data-table/src/utils/convert_value_to_string.ts

Line 24 in 9635cfa

export const convertValueToString = ({

). It's done because the value we render can include html tags (for links and images which users can configure via choosing a formatter for the data view field) and we don't want them to be copied. We want it to be copied as text only. In DocViewer I think it can be achieved if we take row.formattedAsText instead of row.flattenedValue.

[AlexGPlay]

are we ok with mocking some code like copyToClipboard to check that it's getting called and test the code branches depending on its return?

[elasticmachine]

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

[jughosta]

That's a very nice enhancement! Thanks for working on it, @AlexGPlay!

I left some comments but overall looks good!

[jughosta]

Why are we changing this type? It would become easy to forget passing it to the component.

[AlexGPlay]

I changed it because the Copy one doesn't need it but happy to just pass it as undefined

[jughosta]

Since formattedAsText is a custom getter with caching logic, it would be better to access it from onClick handler instead of when Copy component is rendered to prevent unnecessary computations.

[jughosta]

Suggested change

	const Copy: React.FC<TableActionsProps & { toasts: IToasts }> = ({ Component, row, toasts }) => {
	const Copy: React.FC<Omit<TableActionsProps, 'isEsqlMode'> & { toasts: IToasts }> = ({ Component, row, toasts }) => {

Let's drop isEsqlMode for this component in the its type instead.

[AlexGPlay]

sounds good 🙌🏻

[jughosta]

Hi @AlexGPlay,

I have been testing your work and realized that row.formattedAsText does not actually fit for this action. Sorry for misleading!

The problem with it that for most of the fields it returns an array representation as that's the nature of Elasticsearch Fields API. For example, when I see zip value for extension field in the table, the copied value for it is ["zip"]. I checked closely what we have for Copy action inside UnifiedDataTable and we format each item of the array separately and then concatenate.

Let's switch to calling

kibana/src/platform/packages/shared/kbn-unified-data-table/src/utils/convert_value_to_string.ts

Line 24 in 9635cfa

export const convertValueToString = ({

as it's implemented in UnifiedDataTable. I think we could extract this helper to discover-utils package and import it from UnifiedDataTable and UnifiedDocViewer. Also instead of passing rows and rowIndex it could be a rowFlattened value.

[AlexGPlay]

No problem! I'll do that, extract the convertValueToString to discover-utils and change both places to use it - thank you for the review 🙌🏻

[jughosta]

This feature is super helpful, thanks for adding it! LGTM 👍

[elasticmachine]

💚 Build Succeeded

• Buildkite Build
• Commit: 840d148

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
apm	1872	1873	+1
canvas	1307	1308	+1
cloudSecurityPosture	671	672	+1
datasetQuality	431	432	+1
discover	1228	1229	+1
esqlDataGrid	410	411	+1
eventAnnotationListing	579	580	+1
lens	1346	1347	+1
logsShared	337	338	+1
observability	1279	1280	+1
searchPlayground	334	335	+1
securitySolution	7322	7323	+1
slo	1107	1108	+1
unifiedDocViewer	311	312	+1
total			+14

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
@kbn/discover-utils	288	296	+8

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
cloudSecurityPosture	517.2KB	517.4KB	+306.0B
discover	1001.8KB	1002.1KB	+321.0B
esqlDataGrid	155.4KB	155.7KB	+297.0B
securitySolution	9.1MB	9.1MB	+297.0B
slo	908.5KB	908.8KB	+302.0B
unifiedDocViewer	207.7KB	209.3KB	+1.6KB
total			+3.1KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
cloudSecurityPosture	18.0KB	18.0KB	+1.0B
unifiedDocViewer	12.3KB	12.5KB	+167.0B
total			+168.0B

Unknown metric groups

API count

id	before	after	diff
@kbn/discover-utils	338	346	+8

History

• 💚 Build #297528 succeeded d8632b6
• 💛 Build #297456 was flaky 9e6d9e9
• 💚 Build #297196 succeeded 0f448a4
• 💚 Build #296062 succeeded dc6511f
• 💚 Build #295799 succeeded 119008e
• 💛 Build #295648 was flaky cede6ed

cc @AlexGPlay

[janmonschke]

THI changes lgtm

[kibanamachine]

Starting backport for target branches: 8.19

https://github.com/elastic/kibana/actions/runs/14835563650

[kibanamachine]

💔 All backports failed

Status	Branch	Result
❌	8.19	Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

node scripts/backport --pr 218817

Questions ?

Please refer to the Backport tool documentation

[AlexGPlay]

💚 All backports created successfully

Status	Branch	Result
✅	8.19

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation