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

[rollup][licensing_management][grokdebugger] Migrate deprecated EuiPage* components #168300

Conversation

cee-chen
Copy link
Member

@cee-chen cee-chen commented Oct 6, 2023

Summary

Note: I strongly recommending viewing the diff with whitespace changes turned off, as many of the "changes" are simply indentation changes.

EUI will shortly be removing several deprecated EuiPage* components, and we're updating a few remaining Kibana usages of these deprecated components ahead of time.

⚠️ PLEASE NOTE: We have not QA'd these changes to ensure that the UI is 1:1 from before. We do not have the domain knowledge or time to spin up local instances of all plugins with deprecations, and ask that the CODEOWNING team pull down this branch and QA the affected page(s) locally to ensure the UI looks like the same as production. ⚠️

See #161872 for other similar tasks with more information about this effort.

@cee-chen cee-chen added release_note:skip Skip the PR/issue when compiling release notes EUI v8.12.0 labels Oct 6, 2023
@cee-chen cee-chen marked this pull request as ready for review October 6, 2023 23:49
@cee-chen cee-chen requested a review from a team as a code owner October 6, 2023 23:49
@elasticmachine
Copy link
Contributor

Pinging @elastic/eui-team (EUI)

@cee-chen
Copy link
Member Author

cee-chen commented Oct 6, 2023

@elasticmachine merge upstream

@alisonelizabeth alisonelizabeth added the ci:cloud-deploy Create or update a Cloud deployment label Oct 12, 2023
@alisonelizabeth
Copy link
Contributor

@elasticmachine merge upstream

@alisonelizabeth alisonelizabeth added the Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more label Oct 12, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/platform-deployment-management (Team:Deployment Management)

Copy link
Contributor

@alisonelizabeth alisonelizabeth left a comment

Choose a reason for hiding this comment

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

Hi @cee-chen! Thanks for your help on this! Looks like there were a few plugins we missed on our original checklist (#161413) 😬

I did not test License Management (forgot it's disabled on cloud), but I'm seeing a few regressions in Rollups and Grokdebugger. I'm wondering if it'd be easier for an engineer on the Management team to pick this up as they're more familiar with the plugins. That said, we may not be able to get to this for another week or so. WDYT?

@cee-chen
Copy link
Member Author

Apologies for the late response (was on PTO) and thank you so much for the thorough QA @alisonelizabeth! I can take a look at the regressions some time this week, or if y'all want to take over this PR (or branch from it), that'd be totally great too! I have no strong feelings about whoever completes the work as long as it gets done without borking your UIs 😄

@alisonelizabeth
Copy link
Contributor

Apologies for the late response (was on PTO) and thank you so much for the thorough QA @alisonelizabeth! I can take a look at the regressions some time this week, or if y'all want to take over this PR (or branch from it), that'd be totally great too! I have no strong feelings about whoever completes the work as long as it gets done without borking your UIs 😄

Np! If you're up for taking another look at it this week, that would be great. I think we're still a week or so out before we could pick this up.

@cee-chen cee-chen force-pushed the eui-page-deprecated/platform-deployment-management branch from a971985 to 8731782 Compare October 17, 2023 18:06
@cee-chen
Copy link
Member Author

Alrighty, I think I should have addressed all the layout regressions! I switch between the main branch and this one to check that everything visually remained the same as before. I definitely may have missed something however, so please feel free to double check me.

Copy link
Contributor

@alisonelizabeth alisonelizabeth left a comment

Choose a reason for hiding this comment

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

Thanks for working through this @cee-chen! This is looking great.

I only noticed one minor thing with Grokdebugger. In the current UI, the page has an all-white background, but now only the content has the white background. Let me know what you think.

Before:
Screenshot 2023-10-18 at 9 31 42 AM

After:
Screenshot 2023-10-18 at 9 11 01 AM

@cee-chen
Copy link
Member Author

Gah, that's what I get for not QAing on a larger screen. That should be fixed now in 2e4c800 - let me know if not! I also took another quick pass at the other pages/plugins to make sure they didn't have the same problem, and I'm pretty sure it was just that one. 🤞

Copy link
Contributor

@alisonelizabeth alisonelizabeth left a comment

Choose a reason for hiding this comment

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

Looks great now! Thanks again for working on this.

@kibana-ci
Copy link
Collaborator

kibana-ci commented Oct 18, 2023

💚 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
grokdebugger 8.9KB 8.7KB -120.0B
licenseManagement 44.5KB 44.3KB -191.0B
rollup 112.6KB 112.4KB -204.0B
total -515.0B

History

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

@cee-chen cee-chen enabled auto-merge (squash) October 18, 2023 16:56
@cee-chen cee-chen merged commit 121353c into elastic:main Oct 18, 2023
32 checks passed
@cee-chen cee-chen deleted the eui-page-deprecated/platform-deployment-management branch October 18, 2023 17:18
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Oct 18, 2023
benakansara pushed a commit to benakansara/kibana that referenced this pull request Oct 22, 2023
…age*` components (elastic#168300)

## Summary

EUI will shortly be removing several deprecated `EuiPage*` components,
and we're updating a few remaining Kibana usages of these deprecated
components ahead of time.

See elastic#161872 for other similar
tasks with more information about this effort.

---------

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
benakansara pushed a commit to benakansara/kibana that referenced this pull request Oct 22, 2023
…age*` components (elastic#168300)

## Summary

EUI will shortly be removing several deprecated `EuiPage*` components,
and we're updating a few remaining Kibana usages of these deprecated
components ahead of time.

See elastic#161872 for other similar
tasks with more information about this effort.

---------

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
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 ci:cloud-deploy Create or update a Cloud deployment EUI release_note:skip Skip the PR/issue when compiling release notes Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more v8.12.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants