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] expandable flyout - expandable mode only for signal documents #163557

Merged
merged 2 commits into from
Aug 16, 2023

Conversation

PhilippeOberti
Copy link
Contributor

Summary

The expandable flyout is going GA for 8.10, but not all of it. We're release the right panel and a few tabs in the left panel.
Currently the flyout can handle non signal documents, but not gracefully as it was built for alerts specifically. More UI work is needed to correctly handle all other types (event.kind = alert, asset, enrichment, event, metric, state and pipeline_error).

This PR adds a small internal check to the expandable flyout which will hide the Expand details button as well as the Overrview tab if the field event.kind of the viewed document isn't signal.

Screen.Recording.2023-08-10.at.9.40.22.AM.mov

https://github.com/elastic/security-team/issues/6641

>
<ExpandDetailButton />
</div>
{flyoutIsExpandable && (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consider adding some unit tests here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the only reason I didn't add some is because we didn't have any for this file and I was being lazy.. But you're right, I will add them on Monday!

@PhilippeOberti PhilippeOberti force-pushed the expanded-flyout-only-for-signal branch from e80b214 to 2f36733 Compare August 15, 2023 10:21
Copy link
Contributor

@christineweng christineweng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for putting in the unit test. I just have 1 nit comment on the commented line

} from './components/test_ids';
import { mockFlyoutContextValue } from '../shared/mocks/mock_flyout_context';

// jest.mock('../../common/components/link_to');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove commented line

@PhilippeOberti PhilippeOberti force-pushed the expanded-flyout-only-for-signal branch 2 times, most recently from c26a613 to 95595b1 Compare August 15, 2023 21:32
@PhilippeOberti PhilippeOberti force-pushed the expanded-flyout-only-for-signal branch from 95595b1 to fe00ed4 Compare August 16, 2023 08:49
@kibana-ci
Copy link
Collaborator

kibana-ci commented Aug 16, 2023

💔 Build Failed

Failed CI Steps

Test Failures

  • [job] [logs] Serverless Observability Tests / serverless common API Alerting APIs Alerting rules should throttle alerts when appropriate
  • [job] [logs] Serverless Search Tests / serverless common API Alerting APIs Alerting rules should throttle alerts when appropriate
  • [job] [logs] Serverless Security Tests / serverless common API Alerting APIs Alerting rules should throttle alerts when appropriate

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 15.6MB 15.6MB +1.4KB

History

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

@PhilippeOberti PhilippeOberti merged commit f38a37c into main Aug 16, 2023
@PhilippeOberti PhilippeOberti deleted the expanded-flyout-only-for-signal branch August 16, 2023 11:02
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Aug 16, 2023
hop-dev pushed a commit to hop-dev/kibana that referenced this pull request Aug 16, 2023
bryce-b pushed a commit that referenced this pull request Aug 22, 2023
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:enhancement Team:Threat Hunting:Investigations Security Solution Investigations Team v8.10.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants