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

[Enterprise Search] Fix EuiPageTemplate child usages #182711

Merged
merged 2 commits into from
May 9, 2024

Conversation

cee-chen
Copy link
Contributor

@cee-chen cee-chen commented May 6, 2024

👋 Hey y'all ent search folks! I missed this fix/update update in our latest EUI upgrade (#182023). It's directly related to this change in EUI v94.2.0:

  • Fixed an EuiPageTemplate bug where prop updates would not cascade down to child sections (#7648)
    • To cascade props down to the sidebar, EuiPageTemplate now explicitly requires using the EuiPageTemplate.Sidebar rather than EuiPageSidebar

In general, it's also a best practice to use EuiPageTemplate's namespaced children instead of the direct EuiPage* components (which are meant for direct usage within aEuiPage component instead), so I've made similar changes to your code as well. I would suggest pulling down this PR to QA that your page still looks as expected/as before.

@cee-chen
Copy link
Contributor Author

cee-chen commented May 6, 2024

/ci

@cee-chen cee-chen added release_note:skip Skip the PR/issue when compiling release notes v8.15.0 labels May 6, 2024
@cee-chen cee-chen marked this pull request as ready for review May 6, 2024 20:14
@cee-chen cee-chen requested a review from a team May 6, 2024 20:14
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

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
enterpriseSearch 2.7MB 2.7MB -44.0B

History

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

@saarikabhasi
Copy link
Member

saarikabhasi commented May 9, 2024

Thank you @cee-chen for taking care of this.
I did smoke tests of all affected pages and I see changes in side nav in personal dashboard view. Adding a screen recording & screenshot of how current UI will look different with this change:
before

Screen.Recording.2024-05-08.at.10.23.19.PM.mov

after

Screen.Recording.2024-05-08.at.10.22.46.PM.mov

Not found page
before
404 error

after
Screenshot 2024-05-08 at 3 59 01 PM

Overall, I think now layout looks much better 🎉

@cee-chen
Copy link
Contributor Author

cee-chen commented May 9, 2024

Whew! Thanks for checking that and apologies for mildly breaking your layouts in the first place! 🙈

@cee-chen cee-chen merged commit 03a65d2 into elastic:main May 9, 2024
18 checks passed
@cee-chen cee-chen deleted the eui/ent-search-page-template-fix branch May 9, 2024 16:13
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label May 9, 2024
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:skip Skip the PR/issue when compiling release notes v8.15.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants