Skip to content

Flyout state refactor#171696

Closed
PhilippeOberti wants to merge 3 commits intoelastic:mainfrom
PhilippeOberti:flyout_state_refactor
Closed

Flyout state refactor#171696
PhilippeOberti wants to merge 3 commits intoelastic:mainfrom
PhilippeOberti:flyout_state_refactor

Conversation

@PhilippeOberti
Copy link
Copy Markdown
Contributor

@PhilippeOberti PhilippeOberti commented Nov 21, 2023

Summary

This draft PR intends to show an alternative solution to #169661.
It mixes both the old code that was saving the state of the flyout internally in a reducer, and the new code which is purely relying on the url to save the state of the flyout.

Most situation would leverage the url, but some places (like the rule preview) would not want any data to be persisted so would benefit from having the state internally saved in the package.

There are 2 commits:

  • the first one cherry picks the code from @christineweng alert preview PR and squashes them into a single one. This allows us to test the flyout changes with the rule creation page
  • the second one is a squash of all the commits on the original flyout url state PR with some changes to accomodate the rule preview scenario, as well as some cleanup.

Please note the solution presented in this PR isn't finalized. It's more to entertain the discussion around potential solutions of how/where we save the state of the expandable flyout

@kibana-ci
Copy link
Copy Markdown

kibana-ci commented Nov 21, 2023

💔 Build Failed

Failed CI Steps

Test Failures

  • [job] [logs] Jest Tests #1 / rule preview should open preview panel when clicking on button
  • [job] [logs] Jest Tests #1 / should render component
  • [job] [logs] Jest Tests #1 / should open preview panel when clicking on button
  • [job] [logs] Serverless Investigations - Security Solution Cypress Tests #2 / Expandable flyout state sync should test flyout url sync should test flyout url sync
  • [job] [logs] Investigations - Security Solution Cypress Tests #3 / Expandable flyout state sync should test flyout url sync should test flyout url sync
  • [job] [logs] Serverless Investigations - Security Solution Cypress Tests #2 / Expandable flyout state sync should test flyout url sync should test flyout url sync
  • [job] [logs] Investigations - Security Solution Cypress Tests #3 / Expandable flyout state sync should test flyout url sync should test flyout url sync

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 4706 4707 +1

Async chunks

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

id before after diff
securitySolution 12.8MB 12.8MB +2.5KB

Page load bundle

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

id before after diff
securitySolution 65.3KB 65.4KB +39.0B

History

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

@PhilippeOberti PhilippeOberti deleted the flyout_state_refactor branch November 27, 2023 17:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants