[Security Solution] Fixes mis-alignment in the labels of Filter controls on Alerts Page. #192094

logeekal · 2024-09-04T14:27:17Z

Summary

As stated in #192092 , the labels of filter controls were mis-aligned. This was due to recent changes in controls plugin were conflicting with our custom styling. This PR removes the custom styling.

Before

Issues with Error state

It looks like recent PR #190636 on 03-Sept made the controls compact but the error state was left out. This PR fixes that. I would request @elastic/kibana-presentation team to review it once.

After

Below video shows fixes in success, loading and error states. @elastic/response-ops team, Could you please check in other areas as well where these filters are used. Thanks.

filter_controls_labels_fix.mp4

ryankeairns · 2024-09-04T18:12:32Z

packages/kbn-alerts-ui-shared/src/alert_filter_controls/loading.tsx

@@ -12,7 +12,7 @@ import styled from '@emotion/styled';
 import { TEST_IDS } from './constants';

 const FilterGroupLoadingButton = styled(EuiButton)`
-  height: 34px;
+  height: 38px;


I don't believe this is needed either. Instead, try setting a size="s" on line 20. This will set a small button size which seems to match the height of the input controls once they load.

Thanks @ryankeairns .. Great observation. Fixed here : 8e88547

src/plugins/controls/public/react_controls/control_group/components/control_panel.scss

src/plugins/embeddable/public/lib/embeddables/error_embeddable.scss

rshen91

changes LGTM! thank you for the quick fixes!

src/plugins/controls/public/react_controls/control_group/components/control_panel.scss

craig-abbott · 2024-09-05T14:16:42Z

We noticed there was some funky padding happening with a hidden SVG on the same component. Just to confirm, will this PR fix that left side padding issue too? It looks like it does in the screenshot but just wanted to make sure before raising another bug.

Thanks!

logeekal · 2024-09-05T14:49:51Z

packages/kbn-alerts-ui-shared/src/alert_filter_controls/index.scss

-  .euiFormControlLayout.euiFormControlLayout--group.controlFrame__formControlLayout {
-    height: 34px;
-    & .euiFormLabel.controlFrame__formControlLayoutLabel {
-      padding: 8px  !important;


Hey @craig-abbott,

Yes looks like the extra padding appeared after recent changes from presentation team combined with this custom padding we already had here. Now it is fixed since i have removed our custom styles.

However, the padding on the either side of label is still different. But I think it is how @elastic/kibana-presentation wants it to be, since I am using default design of the label. If this needs to be changed, i will prefer to have a new issue raised.

The reason for this padding is that we need to "simulate" left padding to accommodate toggling the drag button when switching between edit mode and view mode (see #184533).

We are currently using a small empty EuiIcon to do this - but now that we have switched to compressed styling, this button is too big. EuiIcon doesn't have an xs option, so we might be forced to just do an empty div with an enforced width of 8px instead (to match the right padding that is now 8px from switching to compressed styling instead of the previous 16px) - what do you think @cqliu1 @rshen91?

Here's what that would look like:

Either way, @logeekal is correct that this is coming from our side and the fix shouldn't be done by ya'll :)

We expect the padding on left and right to be the same, as it is on:
Analytics > Dashboards > [Elastic Security] Detection rule monitoring.

It looks like there, the buttons still using the empty icon there to push the text over, but the padding on the <label> element matches the 12px width.

Whereas in Security > Alerts the icon is 12px but the padding on the right is 8px.

Screenshots for reference:

Apologies @Heenawter it took me so long to gather the screenshots I missed your comment from 12 minutes ago where you explained the problem, and now I look like it just mansplained it back to you!

Great to see collaboration here. Thanks both of you.

@craig-abbott , Could you please re-review the PR and let me know if you have any more comments.

ryankeairns

Thanks for the changes!

logeekal · 2024-09-07T07:52:20Z

@elasticmachine merge upstream

kibana-ci · 2024-09-09T11:41:55Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 1711430

Failed CI Steps

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
`controls`	560.7KB	560.7KB	+24.0B
`securitySolution`	19.7MB	19.7MB	-1.5KB
`triggersActionsUi`	1.6MB	1.6MB	-1.4KB
total			-2.8KB

Page load bundle

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

id	before	after	diff
`embeddable`	71.3KB	71.4KB	+88.0B

History

💔 Build #232566 failed 1390202
💛 Build #232161 was flaky da28fdf
💔 Build #232141 failed 468a37a
💛 Build #231977 was flaky 9569395

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

fixes #192092

9569395

logeekal added release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting v8.16.0 labels Sep 4, 2024

logeekal requested review from a team as code owners September 4, 2024 14:27

logeekal linked an issue Sep 4, 2024 that may be closed by this pull request

[Security Solution] Label of Filter Controls on Alerts page are not aligned. #192092

Closed

ryankeairns requested changes Sep 4, 2024

View reviewed changes

fix: error states and custom styling

8e88547

logeekal commented Sep 5, 2024

View reviewed changes

src/plugins/controls/public/react_controls/control_group/components/control_panel.scss Show resolved Hide resolved

logeekal requested a review from a team September 5, 2024 08:29

fix: more error state styling

468a37a

botelastic bot added the Feature:Embedding Embedding content via iFrame label Sep 5, 2024

logeekal commented Sep 5, 2024

View reviewed changes

src/plugins/embeddable/public/lib/embeddables/error_embeddable.scss Show resolved Hide resolved

fix: types

da28fdf

adcoelho approved these changes Sep 5, 2024

View reviewed changes

rshen91 approved these changes Sep 5, 2024

View reviewed changes

src/plugins/controls/public/react_controls/control_group/components/control_panel.scss Show resolved Hide resolved

logeekal commented Sep 5, 2024

View reviewed changes

Heenawter mentioned this pull request Sep 5, 2024

[Controls] Fix left padding in view mode #192229

Closed

ryankeairns self-requested a review September 6, 2024 17:11

ryankeairns approved these changes Sep 6, 2024

View reviewed changes

Merge branch 'main' into fix/filter_controls_label_alignment

1390202

logeekal enabled auto-merge (squash) September 6, 2024 17:59

Merge branch 'main' into fix/filter_controls_label_alignment

1711430

logeekal merged commit a602eed into main Sep 9, 2024
21 checks passed

logeekal deleted the fix/filter_controls_label_alignment branch September 9, 2024 11:41

logeekal mentioned this pull request Nov 6, 2024

8.16 Elastic Security Release Notes elastic/security-docs#5977

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution] Fixes mis-alignment in the labels of Filter controls on Alerts Page. #192094

[Security Solution] Fixes mis-alignment in the labels of Filter controls on Alerts Page. #192094

logeekal commented Sep 4, 2024 •

edited

Loading

ryankeairns Sep 4, 2024

logeekal Sep 5, 2024

rshen91 left a comment

craig-abbott commented Sep 5, 2024

logeekal Sep 5, 2024

logeekal Sep 5, 2024 •

edited

Loading

Heenawter Sep 5, 2024 •

edited

Loading

craig-abbott Sep 5, 2024

craig-abbott Sep 5, 2024

logeekal Sep 6, 2024

ryankeairns left a comment

logeekal commented Sep 7, 2024

kibana-ci commented Sep 9, 2024

[Security Solution] Fixes mis-alignment in the labels of Filter controls on Alerts Page. #192094

[Security Solution] Fixes mis-alignment in the labels of Filter controls on Alerts Page. #192094

Conversation

logeekal commented Sep 4, 2024 • edited Loading

Summary

Before

Issues with Error state

After

ryankeairns Sep 4, 2024

Choose a reason for hiding this comment

logeekal Sep 5, 2024

Choose a reason for hiding this comment

rshen91 left a comment

Choose a reason for hiding this comment

craig-abbott commented Sep 5, 2024

logeekal Sep 5, 2024

Choose a reason for hiding this comment

logeekal Sep 5, 2024 • edited Loading

Choose a reason for hiding this comment

Heenawter Sep 5, 2024 • edited Loading

Choose a reason for hiding this comment

craig-abbott Sep 5, 2024

Choose a reason for hiding this comment

craig-abbott Sep 5, 2024

Choose a reason for hiding this comment

logeekal Sep 6, 2024

Choose a reason for hiding this comment

ryankeairns left a comment

Choose a reason for hiding this comment

logeekal commented Sep 7, 2024

kibana-ci commented Sep 9, 2024

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Async chunks

Page load bundle

History

logeekal commented Sep 4, 2024 •

edited

Loading

logeekal Sep 5, 2024 •

edited

Loading

Heenawter Sep 5, 2024 •

edited

Loading