Skip to content
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

[Security Solution] make copytoclipboard component more generic #170700

Merged

Conversation

PhilippeOberti
Copy link
Contributor

@PhilippeOberti PhilippeOberti commented Nov 6, 2023

Summary

When we created the CopyToClipboard a few months ago, it was used only in one place, for the Share alert functionality in the flyout's header.
Screenshot 2023-11-07 at 9 33 36 AM

A few weeks ago we used it for the new Json tab. Both UI were similar enough at the time, so only a minor refactor of the component was necessary to support both cases.
Screenshot 2023-11-07 at 9 34 19 AM

Since then the Share alert button has been changed to a single icon (no text). If we still want to use the same shared component, we need it to support both EuiButtonIcon and EuiButtonEmpty as the render is slightly different between the 2 (EuiButtonEmpty has some weird internal padding between the icon and the text, which means that the icon isn't centered when there isn't any text displayed).
Screenshot 2023-11-07 at 9 36 14 AM

This PR redesigns the CopyToClipboard, instead of having it handle multiple renders, it's easier and more flexibile to have the component do the copy functionality only, and let the parent component decide what needs to be rendered.

Checklist

@PhilippeOberti PhilippeOberti marked this pull request as ready for review November 7, 2023 15:37
@PhilippeOberti PhilippeOberti requested a review from a team as a code owner November 7, 2023 15:37
@PhilippeOberti PhilippeOberti added release_note:skip Skip the PR/issue when compiling release notes Team:Threat Hunting:Investigations Security Solution Investigations Team v8.12.0 labels Nov 7, 2023
@PhilippeOberti PhilippeOberti force-pushed the expandable-flyout-copy-to-clipboard branch 2 times, most recently from 730f73c to db61e80 Compare November 14, 2023 03:46
@PhilippeOberti PhilippeOberti force-pushed the expandable-flyout-copy-to-clipboard branch from db61e80 to 86325bc Compare November 14, 2023 19:53
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #22 / security app - OIDC interactions URL capture can login preserving original URL
  • [job] [logs] Serverless Security Cypress Tests #4 / Use Value list in exception entry Should use value list in exception entry, and validate deleting value list prompt Should use value list in exception entry, and validate deleting value list prompt

Metrics [docs]

Async chunks

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

id before after diff
securitySolution 13.0MB 13.0MB -315.0B

History

  • 💚 Build #175548 succeeded db61e80b4155c271abc342fc857ce94b6c5e048d
  • 💔 Build #175520 failed 730f73c00ab419ac504ac0b09e118061640d0689
  • 💛 Build #173674 was flaky 5f38244bf845789ad6c468031f6cd6305b4ad307

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

@PhilippeOberti PhilippeOberti merged commit d1f5a07 into elastic:main Nov 14, 2023
30 checks passed
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Nov 14, 2023
@PhilippeOberti PhilippeOberti deleted the expandable-flyout-copy-to-clipboard branch November 14, 2023 22:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Threat Hunting:Investigations Security Solution Investigations Team v8.12.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants