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

[Fleet] Fix double policy header layout #103076

Conversation

jloleysens
Copy link
Contributor

@jloleysens jloleysens commented Jun 23, 2021

Summary

Fix #102939. Looks like this is a regression from a big refactor: #101828

Current implementation adds an explicit check at the <App /> level for whether we are loading the "Add integration" view and replaces the default header layout (see screenshots).

How to test

  • See the linked issues for details on how to reproduce. This requires starting Elastic+Kibana on basic and setting up an agent for data collection.

  • Also check that existing fleet pages still all look correct while loading. This will require going to the different tabs and pressing refresh while looking for any visual regressions.

To reviewers

  • Is useRouteMatch the correct thing to use here? My testing and understanding of the docs seem to indicate "yes" 🤷🏻‍♂️

Screenshots

Loading "Add integrations"

Notice that tabs from the default layout have been removed for this special case.

Screenshot 2021-06-23 at 13 40 31

"Add integrations" view after these changes are applied

Screenshot 2021-06-23 at 13 43 28

- Use the default page title without tabs while loading the
  add integration view
@jloleysens jloleysens added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v7.14.0 auto-backport Deprecated - use backport:version if exact versions are needed labels Jun 23, 2021
@jloleysens jloleysens requested a review from a team as a code owner June 23, 2021 11:54
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

Copy link
Member

@kpollich kpollich left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@jloleysens
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
fleet 474 476 +2

Async chunks

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

id before after diff
fleet 691.9KB 692.4KB +540.0B

History

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

@jloleysens jloleysens merged commit 9b9c47b into elastic:master Jun 24, 2021
@jloleysens jloleysens deleted the fleet/fix-double-header-in-add-integration branch June 24, 2021 12:45
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Jun 24, 2021
* Fix double policy header layout

- Use the default page title without tabs while loading the
  add integration view

* remove unused import

Co-authored-by: Kibana Machine <[email protected]>
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
7.x

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Jun 24, 2021
* Fix double policy header layout

- Use the default page title without tabs while loading the
  add integration view

* remove unused import

Co-authored-by: Kibana Machine <[email protected]>

Co-authored-by: Jean-Louis Leysens <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v7.14.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Fleet][Integrations] Add integration has double nested layout
4 participants