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

[ML] Response Stream developer example: Migrate deprecated EUI components. #162454

Conversation

walterra
Copy link
Contributor

@walterra walterra commented Jul 25, 2023

Summary

Part of #161408.

Migrates deprecated EUI components for the response stream developer example plugin. To run Kibana with developer examples use yarn start --run-examples. Here's direct links to the pages that have been migrated:

Checklist

@walterra walterra added :ml release_note:skip Skip the PR/issue when compiling release notes v8.10.0 labels Jul 25, 2023
@walterra walterra requested review from darnautov and qn895 July 25, 2023 06:55
@walterra walterra self-assigned this Jul 25, 2023
@walterra walterra requested a review from a team as a code owner July 25, 2023 06:55
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

{children}
</EuiPageContentBody>
</EuiPageContent>
<EuiPageSection style={{ maxWidth: 800, margin: '0 auto' }}>{children}</EuiPageSection>
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you pass a css prop instead? And can we avoid using a magic number? I wonder if there is a variable provided by EUI for page width.

Suggested change
<EuiPageSection style={{ maxWidth: 800, margin: '0 auto' }}>{children}</EuiPageSection>
<EuiPageSection css={{ maxWidth: 800, margin: '0 auto' }}>{children}</EuiPageSection>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Used the opportunity and migrated to EuiPageTemplate instead. That got rid of that custom css altogether and fixes the padding problem on the whole page too!

Before:

image

After:

image

@qn895
Copy link
Member

qn895 commented Jul 27, 2023

Tested and LGTM 🎉

@walterra
Copy link
Contributor Author

Added a breadcrumb to be able to navigate back to the developer examples main page in dab42ee.

developer-example-breadcrumb-0001

Copy link
Contributor

@darnautov darnautov left a comment

Choose a reason for hiding this comment

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

LGTM

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

cc @walterra

@walterra walterra merged commit c2219ba into elastic:main Jul 28, 2023
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Jul 28, 2023
@walterra walterra deleted the ml-161408-migrate-deprecated-eui-components-response-stream branch July 28, 2023 13:42
ThomThomson pushed a commit to ThomThomson/kibana that referenced this pull request Aug 1, 2023
…ents. (elastic#162454)

Migrates deprecated EUI components for the response stream developer
example plugin. Adds a breadcrumb to the example to be able to navigate back to the overview page.
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 :ml release_note:skip Skip the PR/issue when compiling release notes v8.10.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants