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

[SharedUX] Migrate PageTemplate > NoDataPage > NoDataCard #127025

Merged
merged 10 commits into from
Mar 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,5 @@
*/

export * from './elastic_agent_card';
/** @deprecated Use `NoDataCard` from `src/plugins/shared_ux/page_template`. */
export * from './no_data_card';
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

export { NoDataCard } from './no_data_page/no_data_card';

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/
export { NoDataCard } from './no_data_card';
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import React from 'react';
import { NoDataCard } from './no_data_card';
import type { NoDataCardProps } from './types';

export default {
title: 'No Data Card',
description: 'A wrapper around EuiCard, to be used on NoData page',
};

type Params = Pick<NoDataCardProps, 'recommended' | 'button' | 'description'>;

export const PureComponent = (params: Params) => {
return (
<div style={{ width: '50%' }}>
<NoDataCard title={'Add data'} {...params} />
</div>
);
};

PureComponent.argTypes = {
recommended: {
control: 'boolean',
defaultValue: false,
},
button: {
control: {
type: 'text',
},
defaultValue: 'Button text',
},
description: {
control: {
type: 'text',
},
defaultValue: 'This is a description',
},
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import { render } from 'enzyme';
import React from 'react';
import { NoDataCard } from './no_data_card';

describe('NoDataCard', () => {
test('renders', () => {
const component = render(<NoDataCard title="Card title" description="Description" />);
expect(component).toMatchSnapshot();
});

describe('props', () => {
test('recommended', () => {
const component = render(
<NoDataCard recommended title="Card title" description="Description" />
);
expect(component).toMatchSnapshot();
});

test('button', () => {
const component = render(
<NoDataCard button="Button" title="Card title" description="Description" />
);
expect(component).toMatchSnapshot();
});

test('href', () => {
const component = render(
<NoDataCard href="#" button="Button" title="Card title" description="Description" />
);
expect(component).toMatchSnapshot();
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import { i18n } from '@kbn/i18n';
import React, { FunctionComponent } from 'react';
import { EuiButton, EuiCard } from '@elastic/eui';
import type { NoDataCardProps } from './types';

const recommendedLabel = i18n.translate('sharedUX.pageTemplate.noDataPage.recommendedLabel', {
defaultMessage: 'Recommended',
});

const defaultDescription = i18n.translate('sharedUX.pageTemplate.noDataCard.description', {
defaultMessage: `Proceed without collecting data`,
});

export const NoDataCard: FunctionComponent<NoDataCardProps> = ({
recommended,
title,
button,
description,
...cardRest
}) => {
const footer = () => {
if (typeof button !== 'string') {
return button;
}
return <EuiButton fill>{button || title}</EuiButton>;
};
const label = recommended ? recommendedLabel : undefined;
const cardDescription = description || defaultDescription;

return (
<EuiCard
paddingSize="l"
title={title!}
description={cardDescription}
betaBadgeProps={{ label }}
footer={footer()}
{...cardRest}
/>
);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import { EuiCardProps } from '@elastic/eui';
import { MouseEventHandler, ReactNode } from 'react';

export type NoDataCardProps = Partial<Omit<EuiCardProps, 'layout'>> & {
/**
* Applies the `Recommended` beta badge and makes the button `fill`
*/
recommended?: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
recommended?: boolean;
isRecommended?: boolean;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't want to be changing prop names at this time, as I don't know where down the line in PageTemplate is this component used, and renaming them all will add additional work.

/**
* Provide just a string for the button's label, or a whole component
*/
button?: string | ReactNode;
Copy link
Contributor

Choose a reason for hiding this comment

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

@cchaos How does EUI handle this kind of flexibility? Is there a pattern?

@majagrubic Be sure this is the TS type you want to use.

https://dev.to/fromaline/jsxelement-vs-reactelement-vs-reactnode-2mh2
https://github.com/typescript-cheatsheets/react#useful-react-prop-type-examples

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure there's a specific pattern, but the idea here is that the consumer can supply just the button label (string) or replace the whole button their own (in case they don't want to use the same medium, filled button). I guess technically string is included in ReactNode, so maybe that's duplicative. But I think the explicitness here is helpful because an element would replace the whole default EuiButton where the string is just the label. Up to you all if you think there's a better pattern.

Copy link
Contributor

@clintandrewhall clintandrewhall Mar 7, 2022

Choose a reason for hiding this comment

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

I guess I was thinking maybe string | EuiButtonProps.

<Card button="Click Me" />
<Card button={{ label: 'Click Me', onClick: () => {}, fill: true }} />

Then we'd have some control over precisely what component we rendered there, and could force a fill or color to keep the usage consistent, now or in the future.

It's probably minor, but I'm one who leans toward keeping the contract strict at the start, and then expanding outward as necessary, to avoid large-scale refactors later.

Copy link
Contributor

Choose a reason for hiding this comment

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

You could do that. But specifically EuiButton doesn't have a label prop, it's label is just the children, so you'd likely need to append this prop type or rely on consumers to pass children as a key.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh yeah, I think the problem with that, that I'm now remembering, is that you can't go from say an EuiButton to EuiEmptyButton by props alone since they're completely different components. So an example is if the footer button is actually not a CTA we want to promote, or say we need to provide 2 actions instead of just one, there's no way to remove the default button without replacing it completely with a ReactNode.

/**
* Remapping `onClick` to any element
*/
onClick?: MouseEventHandler<HTMLElement>;
/**
* Description for the card. If not provided, the default will be used.
*/
description?: string;
};