[Security Solution][Data Quality Dashboard]: migrate styled-component… #205559

kapral18 · 2025-01-04T02:14:47Z

…s to Emotion

Addresses #205449

This PR migrates ecs_data_quality_dashboard package from styled-components to emotion. In the process we also convert the kbn/ui-theme json tokens to euiTheme counterparts.

Additionally we decorate root babel-jest transform locally in security_solution/public/overview and ecs_data_quality_dashboard package folder to include @emotion/babel-preset-css-prop.

The reason for local babel-jest transforms is that root babel-jest transform doesn't include @emotion/babel-preset-css-prop which is necessary for proper compilation of emotion css prop in tests. Without it there is a warning

appearing in every test that tests a component that uses css prop with theme function passed into it. Other use cases seem to be compiling fine without this babel preset. But theme callback is a valid way of using emotion so we shouldn't avoid using it just because it's not added properly to the test compilation step. Hence I am adding it locally to ecs_data_quality_dashboard package and security_solution/public/overview.

elasticmachine · 2025-01-04T19:50:29Z

Pinging @elastic/security-threat-hunting-explore (Team:Threat Hunting:Explore)

kapral18 · 2025-01-05T01:51:43Z

.../security/packages/ecs_data_quality_dashboard/impl/data_quality_panel/actions/chat/index.tsx

+    linkText: css({
+      display: 'flex',
+      gap: euiTheme.size.xs,
+    }),


using object notation is recommended for 2 reasons:

emotion docs advise to use it for typescript https://emotion.sh/docs/typescript#emotionreact

Free Autocomplete of keys and values.

kapral18 · 2025-01-05T01:53:38Z

...ions/security/packages/ecs_data_quality_dashboard/impl/data_quality_panel/actions/styles.tsx

+export const linkTextCss = ({ euiTheme }: UseEuiTheme): CSSObject => ({
+  display: 'flex',
+  gap: euiTheme.size.s,
+});


Instead of defining shared useStyles hooks unnecessarily, I decided to export small nuggets of shared css snippets like this. When passed directly into css={} prop as a function emotion passes in the theme for free so no need for import, also no need to wrap the properties in css() call.

kapral18 · 2025-01-05T01:55:04Z

...ults/historical_results_list/historical_result/legacy_historical_check_fields/index.test.tsx

@@ -113,7 +113,7 @@ describe('LegacyHistoricalCheckFields', () => {
            <TestExternalProviders>
              <EuiMarkdownFormat>{tablesMarkdown}</EuiMarkdownFormat>
            </TestExternalProviders>
-          ).container.innerHTML
+          ).container.children[0].innerHTML


for some reason the this container started producing an additional <span> I didn't spend time debugging why but I checked that it works as expected visually

kapral18 · 2025-01-05T01:56:03Z

.../data_quality_details/indices_details/pattern/index_check_flyout/index_stats_panel/index.tsx


 export interface Props {
  docsCount: number;
  ilmPhase?: string;
  sizeInBytes?: number;
-  sameFamilyFieldsCount?: number;


cleaning up leftovers from before

kapral18 · 2025-01-05T02:02:57Z

...board/impl/data_quality_panel/data_quality_details/storage_details/storage_treemap/index.tsx

-          className="eui-yScroll"
-          $width={LEGEND_WIDTH}
-        >
+        <div css={styles.legendContainer} data-test-subj="legend" className="eui-scrollBar">


this className change is intentional and is needed because eui-yScroll is added on <style> level and previously <LegendContainer> styled-components styles were also added on style level and took precedence.

But now after replacing with emotion, emotion styles are now coming from compiled css bundle and are being overshadowed by eui-yScroll class's block-size property which completely breaks the height restriction of the container and renders the list unrestricted, so scroll is removed.

I found the eui-scrollBar which is less opinionated and doesn't enforce block-size but applies same basic styling for scrolling https://eui.elastic.co/#/utilities/scroll%23scroll-bar-style I just needed to add overflowY: auto explicitly in this case to achieve parity with eui-yScroll behavior.

kapral18 · 2025-01-05T02:18:02Z

packages/kbn-babel-preset/styled_components_files.js

@@ -25,7 +25,6 @@ module.exports = {
    /x-pack[\/\\]solutions[\/\\]observability[\/\\]plugins[\/\\]synthetics[\/\\]/,
    /x-pack[\/\\]solutions[\/\\]observability[\/\\]plugins[\/\\]uptime[\/\\]/,
    /x-pack[\/\\]solutions[\/\\]observability[\/\\]plugins[\/\\]ux[\/\\]/,
-    /x-pack[\/\\]solutions[\/\\]security[\/\\]packages[\/\\]ecs_data_quality_dashboard[\/\\]/,


this line was the puzzle piece that helped me debug what happened before with my faulty setup last time I tried to convert to emotion and failed. Turns out we have to uncomment this line to allow proper emotion compilation on webpack side. Now everything works as expected.

cc: @semd @angorayc @agusruidiazgd

kapral18 · 2025-01-05T13:24:33Z

x-pack/solutions/security/plugins/security_solution/public/common/mock/test_providers.tsx

-      </MockKibanaContextProvider>
-    </I18nProvider>
+    <MockKibanaContextProvider startServices={startServices}>
+      <KibanaRenderContextProvider {...coreMock.createStart()}>


this turned out to cause issues with existing tests, so I moved this to data_quality.test only for now with latest commit

…s to Emotion Addresses elastic#205449 - Replaced styled-components with Emotion for styling components - Updated all styled-components imports to Emotion - Refactored styles using Emotion's `css` and `useEuiTheme` hooks - Ensured all components are using Emotion's `css` prop for styling - Updated tsconfig to include Emotion types and configuration - Removed ecs_data_quality_dashboard package from packages/kbn-babel-preset/styled_components_files.js so that emotion is properly compiled

The whole point of this change is to make babel-jest transform options shared and reusable by other packages' jest.config transforms that need to add their own custom stuff. For example here ecs_data_quality_dashboard makes use of that to add @emotion/babel-preset-css-prop that is needed to properly compile emotion css prop in jest tests.

…fix'

- Add a new babel transformer for jest in the security solution plugin to enable overview/data_quality.test.tsx to pass without compilation warnings - Integrate `@emotion/babel-preset-css-prop` with custom label format - Update `test_providers.tsx` to include `KibanaRenderContextProvider` and `coreMock` - Modify jest configuration in `overview` to use the new babel transformer - Ensure compatibility with existing jest preset and module name mapping

- Removed KibanaRenderContextProvider from TestProvidersComponent due to issues in current overview tests - Updated test setup in data_quality.test.tsx to include KibanaRenderContextProvider instead

…apping'

kapral18 · 2025-01-05T20:08:02Z

/ci

elasticmachine · 2025-01-05T21:44:23Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 896cf38

Failed CI Steps

Post-Build

Metrics [docs]

‼️ ERROR: no builds found for mergeBase sha [211d4a6]

History

💔 Build #264193 failed 896cf38
💔 Build #264187 failed 896cf38
💔 Build #264182 failed c4ebd35
💔 Build #264181 failed c4ebd35
💔 Build #264179 failed b7d2a9d
💔 Build #264175 failed 80a70e3

cc @kapral18

pheyos

kbn-test changes LGTM

KDKHD · 2025-01-06T08:28:26Z

x-pack/solutions/security/plugins/security_solution/public/common/mock/test_providers.tsx

+    <MockKibanaContextProvider startServices={startServices}>
+      <I18nProvider>


How come these needed to be swapped?

good question. Initially I checked the root of security_solution app and it was wrapped in KibanaContextProvider without anything above it. But now after double checking I see that I18nProvider is not even present anywhere, only in tests. But we can clean it up later wdyt?

Sure, can be cleaned up later. You already have 2 approvals here :)

KDKHD

✅ for security_solution files!

kibanamachine · 2025-01-06T13:48:33Z

Starting backport for target branches: 8.x

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

kibanamachine · 2025-01-06T13:53:41Z

💔 All backports failed

Status	Branch	Result
❌	8.x	Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

node scripts/backport --pr 205559

Questions ?

Please refer to the Backport tool documentation

kapral18 self-assigned this Jan 4, 2025

kapral18 force-pushed the feat/data-quality/205449-migrate-to-emotion branch 3 times, most recently from c5df37c to b91c385 Compare January 4, 2025 17:57

kapral18 marked this pull request as ready for review January 4, 2025 19:48

kapral18 requested review from a team as code owners January 4, 2025 19:48

kapral18 added release_note:skip Skip the PR/issue when compiling release notes backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) Team:Threat Hunting:Explore labels Jan 4, 2025

kapral18 force-pushed the feat/data-quality/205449-migrate-to-emotion branch 2 times, most recently from 33c4085 to 4d0d6da Compare January 5, 2025 01:33

kapral18 commented Jan 5, 2025

View reviewed changes

kapral18 force-pushed the feat/data-quality/205449-migrate-to-emotion branch from 80a70e3 to b7d2a9d Compare January 5, 2025 11:46

kapral18 requested a review from a team as a code owner January 5, 2025 11:46

kapral18 commented Jan 5, 2025

View reviewed changes

kapral18 force-pushed the feat/data-quality/205449-migrate-to-emotion branch from c4ebd35 to 90240b3 Compare January 5, 2025 15:53

kapral18 and others added 5 commits January 5, 2025 17:01

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

153dcfd

…fix'

refactor: remove KibanaRenderContextProvider and update test setup

5ab5bfe

- Removed KibanaRenderContextProvider from TestProvidersComponent due to issues in current overview tests - Updated test setup in data_quality.test.tsx to include KibanaRenderContextProvider instead

kapral18 force-pushed the feat/data-quality/205449-migrate-to-emotion branch from 90240b3 to 5ab5bfe Compare January 5, 2025 16:03

[CI] Auto-commit changed files from 'node scripts/styled_components_m…

896cf38

…apping'

kapral18 added ci:cloud-deploy Create or update a Cloud deployment ci:cloud-persist-deployment Persist cloud deployment indefinitely ci:project-deploy-security Create a Security Serverless Project ci:project-persist-deployment Persist project deployment indefinitely labels Jan 5, 2025

pheyos approved these changes Jan 6, 2025

View reviewed changes

KDKHD reviewed Jan 6, 2025

View reviewed changes

KDKHD approved these changes Jan 6, 2025

View reviewed changes

delanni approved these changes Jan 6, 2025

View reviewed changes

kapral18 removed ci:cloud-deploy Create or update a Cloud deployment ci:cloud-persist-deployment Persist cloud deployment indefinitely ci:project-deploy-security Create a Security Serverless Project ci:project-persist-deployment Persist project deployment indefinitely labels Jan 6, 2025

angorayc approved these changes Jan 6, 2025

View reviewed changes

kapral18 merged commit 72980e1 into elastic:main Jan 6, 2025
20 checks passed

kapral18 deleted the feat/data-quality/205449-migrate-to-emotion branch January 6, 2025 13:48

kibanamachine added the v9.0.0 label Jan 6, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution][Data Quality Dashboard]: migrate styled-component… #205559

[Security Solution][Data Quality Dashboard]: migrate styled-component… #205559

kapral18 commented Jan 4, 2025 •

edited by kibanamachine

Loading

elasticmachine commented Jan 4, 2025

kapral18 Jan 5, 2025

kapral18 Jan 5, 2025 •

edited

Loading

kapral18 Jan 5, 2025

kapral18 Jan 5, 2025

kapral18 Jan 5, 2025

kapral18 Jan 5, 2025

kapral18 Jan 5, 2025

kapral18 commented Jan 5, 2025

elasticmachine commented Jan 5, 2025

pheyos left a comment

KDKHD Jan 6, 2025

kapral18 Jan 6, 2025

KDKHD Jan 6, 2025

KDKHD left a comment

kibanamachine commented Jan 6, 2025

kibanamachine commented Jan 6, 2025

		<MockKibanaContextProvider startServices={startServices}>
		<I18nProvider>

[Security Solution][Data Quality Dashboard]: migrate styled-component… #205559

[Security Solution][Data Quality Dashboard]: migrate styled-component… #205559

Conversation

kapral18 commented Jan 4, 2025 • edited by kibanamachine Loading

elasticmachine commented Jan 4, 2025

Choose a reason for hiding this comment

kapral18 Jan 5, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kapral18 commented Jan 5, 2025

elasticmachine commented Jan 5, 2025

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

History

pheyos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KDKHD left a comment

Choose a reason for hiding this comment

kibanamachine commented Jan 6, 2025

kibanamachine commented Jan 6, 2025

💔 All backports failed

Manual backport

Questions ?

kapral18 commented Jan 4, 2025 •

edited by kibanamachine

Loading

kapral18 Jan 5, 2025 •

edited

Loading