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

[Watcher] Migrate all usages of EuiPage*_Deprecated #163128

Merged
merged 6 commits into from
Aug 22, 2023

Conversation

gitstart
Copy link
Contributor

@gitstart gitstart commented Aug 3, 2023

What does this PR do?

  • Migrate all usages of EuiPage*_Deprecated in Watcher

Issue References

Video/Screenshot Demo


This code was written and reviewed by GitStart Community. Growing great engineers, one PR at a time.

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

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@gitstart gitstart changed the title ELS-160 - [Watcher] Migrate all usages of EuiPage*_Deprecated [Watcher] Migrate all usages of EuiPage*_Deprecated Aug 3, 2023
@gitstart gitstart marked this pull request as ready for review August 3, 2023 22:55
@gitstart gitstart requested a review from a team as a code owner August 3, 2023 22:55
@alisonelizabeth
Copy link
Contributor

Hi @gitstart! Thank you for your contribution ❤️

Can you please provide screenshots of the changes you have made? This will help us greatly with the review and confirm the changes have been tested.

Also - a quick note: While the “How” section in #161413 provides a quick conversion map, unfortunately, this does not work for all cases. Please refer to the EUI guidelines on how to approach the migration and examples. You can also take a look at this PR as a reference implementation: #163134.

Please let us know if you have any questions. Thanks!

Copy link
Contributor

@yuliacech yuliacech left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the help, @gitstart! Please feel free to re-request a code review from the team platform-deployment-management after merge conflicts and feedback in the comment have been addressed.

@yuliacech yuliacech added Feature:Watcher Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more release_note:skip Skip the PR/issue when compiling release notes labels Aug 17, 2023
@elasticmachine
Copy link
Contributor

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

Co-authored-by: LuisChiej <[email protected]>
Co-authored-by: gitstart_bot <[email protected]>
@gitstart gitstart requested a review from yuliacech August 22, 2023 08:12
import { FormattedMessage } from '@kbn/i18n-react';

export function PageErrorForbidden() {
return (
<EuiEmptyPrompt
<EuiPageTemplate.EmptyPrompt
Copy link
Contributor

Choose a reason for hiding this comment

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

Verified ✅
Screenshot 2023-08-22 at 13 11 02

import { FormattedMessage } from '@kbn/i18n-react';

export function PageErrorNotExist({ id }: { id?: string }) {
return (
<EuiEmptyPrompt
<EuiPageTemplate.EmptyPrompt
Copy link
Contributor

Choose a reason for hiding this comment

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

Verified ✅
Screenshot 2023-08-22 at 13 13 08

actions={[promptAction]}
/>
</EuiPageContent>
<EuiPageTemplate.EmptyPrompt
Copy link
Contributor

Choose a reason for hiding this comment

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

Verified ✅
Screenshot 2023-08-22 at 13 14 05

@@ -94,7 +90,7 @@ export const JsonWatchEdit = ({ pageTitle }: { pageTitle: string }) => {
);

return (
<EuiPageContentBody restrictWidth style={{ width: '100%' }}>
<EuiPageSection restrictWidth style={{ width: '100%' }}>
Copy link
Contributor

Choose a reason for hiding this comment

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

Verified ✅
Screenshot 2023-08-22 at 13 16 01

@@ -236,7 +236,7 @@ export const ThresholdWatchEdit = ({ pageTitle }: { pageTitle: string }) => {
};

return (
<EuiPageContentBody restrictWidth style={{ width: '100%' }}>
<EuiPageSection restrictWidth style={{ width: '100%' }}>
Copy link
Contributor

Choose a reason for hiding this comment

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

Verified ✅
Screenshot 2023-08-22 at 13 17 38

<PageError errorCode={errorCode} id={id} />
</EuiPageContent>
);
return <PageError errorCode={errorCode} id={id} />;
Copy link
Contributor

Choose a reason for hiding this comment

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

Verified ✅
Screenshot 2023-08-22 at 13 29 44

@@ -154,14 +150,14 @@ export const WatchEditPage = ({

if (!watch) {
return (
<EuiPageContent verticalPosition="center" horizontalPosition="center" color="subdued">
<EuiPageTemplate.EmptyPrompt>
Copy link
Contributor

Choose a reason for hiding this comment

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

Added EuiPageTemplate.EmptyPrompt otherwise the loading prompt doesn't have the color.
Screenshot 2023-08-22 at 13 33 51

@@ -178,24 +177,20 @@ export const WatchListPage = () => {

if (isWatchesLoading) {
return (
<EuiPageContent verticalPosition="center" horizontalPosition="center" color="subdued">
<EuiPageTemplate.EmptyPrompt>
Copy link
Contributor

Choose a reason for hiding this comment

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

Added EuiPageTemplate.EmptyPrompt otherwise the loading section doesn't have the color.
Screenshot 2023-08-22 at 13 36 41

<PageError errorCode={errorCode} />
</EuiPageContent>
);
return <PageError errorCode={errorCode} />;
Copy link
Contributor

Choose a reason for hiding this comment

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

Verified ✅
Screenshot 2023-08-22 at 13 38 01

data-test-subj="emptyPrompt"
/>
</EuiPageContent>
<EuiPageTemplate.EmptyPrompt
Copy link
Contributor

Choose a reason for hiding this comment

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

Verified ✅
Screenshot 2023-08-22 at 13 38 54

@@ -77,23 +77,19 @@ export const WatchStatusPage = ({

if (isWatchDetailLoading) {
return (
<EuiPageContent verticalPosition="center" horizontalPosition="center" color="subdued">
<EuiPageTemplate.EmptyPrompt>
Copy link
Contributor

Choose a reason for hiding this comment

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

Added EuiPageTemplate.EmptyPrompt otherwise the loading prompt doesn't have the color.
Screenshot 2023-08-22 at 13 40 40

<PageError errorCode={errorCode} id={id} />
</EuiPageContent>
);
return <PageError errorCode={errorCode} id={id} />;
Copy link
Contributor

Choose a reason for hiding this comment

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

Verified ✅
Screenshot 2023-08-22 at 13 42 02

]}
/>
</EuiPageContent>
<EuiPageTemplate.EmptyPrompt
Copy link
Contributor

Choose a reason for hiding this comment

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

didn't verify this visually since I'm not sure, monitoring watches are still in use. The code changes LGTM

@yuliacech
Copy link
Contributor

buildkite test this

Copy link
Contributor

@yuliacech yuliacech left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the help, @gitstart!
Verified visually and added some fixes, the rest of the changes LGTM 👍

@yuliacech
Copy link
Contributor

buildkite test this

@yuliacech
Copy link
Contributor

@elasticmachine run elasticsearch-ci/docs

@yuliacech yuliacech enabled auto-merge (squash) August 22, 2023 12:33
@yuliacech yuliacech merged commit 34733be into elastic:main Aug 22, 2023
@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
securitySolution 15.7MB 15.7MB +6.4KB
securitySolutionServerless 249.0KB 255.3KB +6.2KB
watcher 163.9KB 163.1KB -801.0B
total +11.9KB

Page load bundle

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

id before after diff
securitySolutionServerless 37.9KB 37.9KB +2.0B

History

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

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 💝community Feature:Watcher 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.11.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants