Skip to content

Replace EuiPanel with EuiCard when using beta badges#96147

Merged
smith merged 6 commits intoelastic:masterfrom
smith:nls/beta-badge-card
Apr 7, 2021
Merged

Replace EuiPanel with EuiCard when using beta badges#96147
smith merged 6 commits intoelastic:masterfrom
smith:nls/beta-badge-card

Conversation

@smith
Copy link
Contributor

@smith smith commented Apr 2, 2021

In elastic/eui#4649 the betaBadgeLabel and related props have been removed from EuiPanel and it's now recommended to use an EuiCard instead.

Replace these in APM and Observability plugins and update stories so examples can be viewed.

License Prompt

Before After
image image

Fleet Panel (observability)

Before After
image image

Custom link preview

Before After
image image

Storybook examples

(Press A in the Storybook preview to open the controls panel to adjust component props)

In elastic/eui#4649 the `betaBadgeLabel` and related props have been removed from `EuiPanel` and it's now recommended to use an `EuiCard` instead.

Replace these in APM and Observability plugins and update stories so examples can be viewed.
@smith smith added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.13.0 auto-backport Deprecated - use backport:version if exact versions are needed labels Apr 2, 2021
@smith smith requested a review from a team April 2, 2021 15:23
@botelastic botelastic bot added the Team:APM - DEPRECATED Use Team:obs-ux-infra_services. label Apr 2, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/apm-ui (Team:apm)

Copy link
Contributor

@cauemarcondes cauemarcondes left a comment

Choose a reason for hiding this comment

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

LGTM, could you add some screenshots?

Copy link
Contributor

@cchaos cchaos left a comment

Choose a reason for hiding this comment

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

I've sending a couple more suggestions. Really I think there are some overrides of EuiCard that are unnecessary and/or places where the card isn't really appropriate anyway.

Comment on lines +37 to +41
title={
<EuiTitle size="s">
<h4>
{i18n.translate('xpack.observability.fleet.title', {
defaultMessage: 'Have you seen our new Fleet?',
Copy link
Contributor

Choose a reason for hiding this comment

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

The title is already wrapped in an EuiTitle so what you really want is just:

Suggested change
title={
<EuiTitle size="s">
<h4>
{i18n.translate('xpack.observability.fleet.title', {
defaultMessage: 'Have you seen our new Fleet?',
titleElement="h4"
title={i18n.translate('xpack.observability.fleet.title', {
defaultMessage: 'Have you seen our new Fleet?',

{i18n.translate('xpack.observability.fleet.button', {
defaultMessage: 'Try Fleet Beta',
description={
<EuiText size="s" color="subdued" style={{ maxWidth: '700px' }}>
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this will actually max out the card? I'd apply this style tag to the actual EuiCard style={}.

Also, the description is already wrapped in a small EuiText, so all you need is the text color

Suggested change
<EuiText size="s" color="subdued" style={{ maxWidth: '700px' }}>
<EuiTextColor color="subdued">

)}
</EuiText>
<EuiSpacer />
<EuiCard
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this conversion if forcing EuiCard to be something that it is not. My honest suggestion would be to just move the "Preview" text to a heading similar to the rest of the sections with: <EuiTitle size="xs"><h3 /></EuiTitle> before the panel.

@smith
Copy link
Contributor Author

smith commented Apr 2, 2021

LGTM, could you add some screenshots?

@cauemarcondes at the moment there aren't visual changes, so everything basically looks the same as before. I was planning on linking to the stories once they're built. I'll do that once I address @cchaos's feedback.

@kibanamachine
Copy link
Contributor

💚 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
apm 4.2MB 4.2MB +254.0B
observability 412.0KB 411.4KB -657.0B
total -403.0B

History

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

@smith smith merged commit 8d2d2ad into elastic:master Apr 7, 2021
@smith smith deleted the nls/beta-badge-card branch April 7, 2021 21:51
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Apr 7, 2021
In elastic/eui#4649 the `betaBadgeLabel` and related props have been removed from `EuiPanel` and it's now recommended to use an `EuiCard` instead.

Replace these in APM and Observability plugins and update stories so examples can be viewed.
@kibanamachine
Copy link
Contributor

💚 Backport successful

7.x / #96512

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Apr 8, 2021
…6512)

In elastic/eui#4649 the `betaBadgeLabel` and related props have been removed from `EuiPanel` and it's now recommended to use an `EuiCard` instead.

Replace these in APM and Observability plugins and update stories so examples can be viewed.

Co-authored-by: Nathan L Smith <nathan.smith@elastic.co>
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:APM - DEPRECATED Use Team:obs-ux-infra_services. v7.13.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants