[ML] Migrate anomaly explorer components from SCSS to Emotion #212793

peteharverson · 2025-02-28T16:32:26Z

Summary

Part of: #140695
Migrates SCSS to emotion for several of the components used across the Anomaly Explorer and Single Metric Viewer.

Removes the following SCSS files:

- x-pack/platform/plugins/shared/m/public/application/components/annotations/annotation_description_list/_index.scss
- x-pack/platform/plugins/shared/ml/public/application/components/entity_cell/_index.scss
- x-pack/platform/plugins/shared/ml/public/application/components/entity_cell/entity_cell.scss
- x-pack/platform/plugins/shared/ml/public/application/components/help_popover/help_popover.scss
- x-pack/platform/plugins/shared/ml/public/application/components/detector_description_list/_detector_description_list.scss
- x-pack/platform/plugins/shared/ml/public/application/components/rule_editor/components/detector_description_list/_index.scss
- x-pack/platform/plugins/shared/ml/public/application/explorer/explorer_charts/components/explorer_chart_label/_explorer_chart_label_badge.scss
- x-pack/platform/plugins/shared/ml/public/application/explorer/explorer_charts/components/explorer_chart_label/entity_filter/_entity_filter.scss

Components edited:

Help popover

Annotation description list:

Rule editor detector description list:

Anomalies table entity filters:

Explorer chart label badge and entity filters:

elasticmachine · 2025-02-28T16:32:29Z

Pinging @elastic/ml-ui (:ml)

rbrtj · 2025-03-03T09:26:03Z

...latform/plugins/shared/ml/public/application/components/anomalies_table/influencers_cell.tsx

+              <EuiButtonIcon
+                size="s"
+                css={filterButton}
+                onClick={blurButtonOnClick(() => {
+                  influencerFilter(
+                    influencer.influencerFieldName,
+                    influencer.influencerFieldValue,
+                    '+'
+                  );
+                })}
+                iconType="plusInCircle"
+                aria-label={i18n.translate(
+                  'xpack.ml.anomaliesTable.influencersCell.addFilterAriaLabel',
+                  {
+                    defaultMessage: 'Add filter',
+                  }
+                )}
+              />


I think the influencers filter should be visible only if the influencerFilter prop is defined. Currently, we always see the buttons, but clicking them throws an error in the console:

Done in cafbf1e

rbrtj · 2025-03-03T09:58:59Z

...latform/plugins/shared/ml/public/application/components/anomalies_table/influencers_cell.tsx

+  const { filterButton } = useEntityCellStyles();
+
+  const [showAllInfluencers, setShowAllInfluencers] = useState(false);
+  const toggleAllInfluencers = setShowAllInfluencers.bind(null, (prev) => !prev);


I'm not sure if bind is necessary here

Done in cafbf1e

rbrtj · 2025-03-03T10:14:11Z

x-pack/platform/plugins/shared/ml/public/application/components/help_popover/help_popover.tsx

@@ -10,12 +10,12 @@ import React, { useState } from 'react';
 import { i18n } from '@kbn/i18n';
 import type { EuiLinkButtonProps, EuiPopoverProps } from '@elastic/eui';
 import { EuiButtonIcon, EuiPopover, EuiPopoverTitle, EuiText } from '@elastic/eui';
-import './help_popover.scss';
+// import './help_popover.scss';


Suggested change

// import './help_popover.scss';

Removed in cafbf1e

rbrtj · 2025-03-03T10:27:22Z

x-pack/platform/plugins/shared/ml/public/application/components/rule_editor/_rule_editor.scss

@@ -2,10 +2,6 @@
 .ml-rule-editor-flyout {
  font-size: $euiFontSizeS;

-  .select-rule-action .rule-detector-description-list {


As the select-rule-action is removed, we should also remove it in:
https://github.com/elastic/kibana/blob/main/x-pack/platform/plugins/shared/ml/public/application/components/rule_editor/select_rule_action/select_rule_action.js#L53

Done in cafbf1e

rbrtj · 2025-03-03T10:35:54Z

...plorer/explorer_charts/components/explorer_chart_label/entity_filter/entity_filter_styles.ts

NIT: this is the same as the filterButton style in x-pack/platform/plugins/shared/ml/public/application/components/entity_cell/entity_cell_styles.ts. Maybe it could be extracted into something like filter_button_styles

Agree that this would be the ideal approach, with some refactoring to create a separate component for the filter button which is used in several places. But I want to keep the scope of this PR small, and save that refactoring for future work.

peteharverson · 2025-03-04T09:40:45Z

@elasticmachine merge upstream

jgowdyelastic

Left a nitpick, but

jgowdyelastic · 2025-03-04T09:14:07Z

x-pack/platform/plugins/shared/ml/public/application/components/entity_cell/entity_cell.tsx

@@ -53,10 +55,13 @@ function getAddFilter({ entityName, entityValue, filter }: EntityCellProps) {
        />
      </EuiToolTip>
    );
+  } else {


Nit, we usually return early with null rather than have it in an else as it is easier to read.
e.g.

if (filter === undefined) { return null; } return ( .....

Done in ac5e22e

jgowdyelastic · 2025-03-04T09:15:03Z

x-pack/platform/plugins/shared/ml/public/application/components/entity_cell/entity_cell.tsx

@@ -81,8 +86,10 @@ function getRemoveFilter({ entityName, entityValue, filter }: EntityCellProps) {
        />
      </EuiToolTip>
    );
+  } else {
+    return null;


same comment as above

Done in ac5e22e

jgowdyelastic · 2025-03-04T10:01:19Z

...plorer/explorer_charts/components/explorer_chart_label/entity_filter/entity_filter_styles.ts

+      opacity: 0.5,
+      width: euiTheme.size.base,
+      height: euiTheme.size.base,
+      '-webkit-transform': 'translateY(-1px)',


I'm seeing errors in the console which are being caused by these -webkit-transform rules.

The errors do not appear if these are removed.

Removed in ac5e22e

jgowdyelastic

LGTM

elasticmachine · 2025-03-05T14:19:16Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: ac5e22e

Failed CI Steps

FTR Configs #2

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`ml`	2337	2322	-15

Async chunks

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

id	before	after	diff
`ml`	5.2MB	5.2MB	-11.9KB

History

💚 Build #280637 succeeded ae501a0
💛 Build #280438 was flaky 69feaa4
💔 Build #280392 failed 0fee3fb
💔 Build #280030 failed 45847d9

cc @peteharverson

ryankeairns

SCSS changes

rbrtj

Latest changes LGTM 👍

kibanamachine · 2025-03-05T17:27:29Z

Starting backport for target branches: 8.x

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

kibanamachine · 2025-03-05T17:33:16Z

💔 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 212793

Questions ?

Please refer to the Backport tool documentation

peteharverson · 2025-03-06T10:43:13Z

💚 All backports created successfully

Status	Branch	Result
✅	8.x

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

Questions ?

Please refer to the Backport tool documentation

[ML] Migrate anomaly explorer components from SCSS to Emotion

45847d9

peteharverson added chore :ml release_note:skip Skip the PR/issue when compiling release notes backport:version Backport to applied version labels v9.1.0 v8.19.0 labels Feb 28, 2025

peteharverson requested review from jgowdyelastic and rbrtj February 28, 2025 16:32

peteharverson self-assigned this Feb 28, 2025

peteharverson requested review from a team as code owners February 28, 2025 16:32

peteharverson mentioned this pull request Feb 28, 2025

[ML] Replace legacy SCSS overwrites with css-in-js #140695

Open

68 tasks

rbrtj reviewed Mar 3, 2025

View reviewed changes

peteharverson added 3 commits March 3, 2025 16:03

[ML] Fix translations. Address review comments.

cafbf1e

[ML] Remove comment

0fee3fb

[ML] Fix explorer_chart_label_badge jest test.

69feaa4

Merge branch 'main' into ml-anomaly-explorer-scss-migration

ae501a0

jgowdyelastic reviewed Mar 4, 2025

View reviewed changes

peteharverson mentioned this pull request Mar 4, 2025

[ML] Migrate Javascript files to Typescript #153894

Open

40 tasks

[ML] Address review comments

ac5e22e

jgowdyelastic approved these changes Mar 5, 2025

View reviewed changes

ryankeairns approved these changes Mar 5, 2025

View reviewed changes

rbrtj approved these changes Mar 5, 2025

View reviewed changes

peteharverson merged commit a1c520c into elastic:main Mar 5, 2025
9 checks passed

peteharverson deleted the ml-anomaly-explorer-scss-migration branch March 5, 2025 17:27

peteharverson mentioned this pull request Mar 6, 2025

[8.x] [ML] Migrate anomaly explorer components from SCSS to Emotion (#212793) #213352

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ML] Migrate anomaly explorer components from SCSS to Emotion #212793

[ML] Migrate anomaly explorer components from SCSS to Emotion #212793

peteharverson commented Feb 28, 2025 •

edited by kibanamachine

Loading

elasticmachine commented Feb 28, 2025

rbrtj Mar 3, 2025

peteharverson Mar 3, 2025

rbrtj Mar 3, 2025

peteharverson Mar 3, 2025

rbrtj Mar 3, 2025

peteharverson Mar 3, 2025

rbrtj Mar 3, 2025

peteharverson Mar 3, 2025

rbrtj Mar 3, 2025

peteharverson Mar 4, 2025

peteharverson commented Mar 4, 2025

jgowdyelastic left a comment

jgowdyelastic Mar 4, 2025

peteharverson Mar 5, 2025

jgowdyelastic Mar 4, 2025

peteharverson Mar 5, 2025

jgowdyelastic Mar 4, 2025

peteharverson Mar 5, 2025

jgowdyelastic left a comment

elasticmachine commented Mar 5, 2025

ryankeairns left a comment

rbrtj left a comment

kibanamachine commented Mar 5, 2025

kibanamachine commented Mar 5, 2025

peteharverson commented Mar 6, 2025

[ML] Migrate anomaly explorer components from SCSS to Emotion #212793

[ML] Migrate anomaly explorer components from SCSS to Emotion #212793

Conversation

peteharverson commented Feb 28, 2025 • edited by kibanamachine Loading

Summary

elasticmachine commented Feb 28, 2025

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

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

peteharverson commented Mar 4, 2025

jgowdyelastic 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jgowdyelastic left a comment

Choose a reason for hiding this comment

elasticmachine commented Mar 5, 2025

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Module Count

Async chunks

History

ryankeairns left a comment

Choose a reason for hiding this comment

rbrtj left a comment

Choose a reason for hiding this comment

kibanamachine commented Mar 5, 2025

kibanamachine commented Mar 5, 2025

💔 All backports failed

Manual backport

Questions ?

peteharverson commented Mar 6, 2025

💚 All backports created successfully

Questions ?

peteharverson commented Feb 28, 2025 •

edited by kibanamachine

Loading