[SharedUX] Migrate PageTemplate > NoData Page #128372

majagrubic · 2022-03-23T13:44:01Z

Summary

This PR migrates NoDataPage from kibana_react to shared_ux. After an offline sync with @cchaos, there are a few notable changes:

The page is intended to only render one card, not an array of cards as previously; all the props and logic have been updated to follow this
The card is going to exclusively be ElasticAgentCard, not NoDataCard as previously. This has also been updated in props definition (but leaving NoData card now in the repo for now, will be removed in a subsequent PR)
recommended prop on the card is not needed and it won't be used

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk	Probability	Severity	Mitigation/Notes
Multiple Spaces—unexpected behavior in non-default Kibana Space.	Low	High	Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces.
Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks.	High	Low	Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure.
Code should gracefully handle cases when feature X or plugin Y are disabled.	Medium	High	Unit tests will verify that any feature flag or plugin combination still results in our service operational.
See more potential risk examples

For maintainers

This was checked for breaking API changes and was labeled appropriately

majagrubic · 2022-03-28T11:29:35Z

@elasticmachine merge upstream

cchaos · 2022-03-30T20:13:39Z

packages/kbn-shared-ux-components/src/page_template/no_data_page/types.ts

+export type NoDataPageActions = Partial<EuiCardProps> & {
+  /**
+   * Provide just a string for the button's label, or a whole component
+   */
+  button?: string | ReactNode;
+  /**
+   * Remapping `onClick` to any element
+   */
+  onClick?: MouseEventHandler<HTMLElement>;
+  /**
+   * Category to auto-select within Fleet
+   */
+  category?: string;
+};


These are actually extending ElasticAgentCardProps without the description. Maybe we can just extend otherwise we'll need to keep the props and descriptions in sync?

Suggested change

export type NoDataPageActions = Partial<EuiCardProps> & {

/**

* Provide just a string for the button's label, or a whole component

*/

button?: string | ReactNode;

/**

* Remapping `onClick` to any element

*/

onClick?: MouseEventHandler<HTMLElement>;

/**

* Category to auto-select within Fleet

*/

category?: string;

};

export type NoDataPageActions = Omit<ElasticAgentCardProps, 'description'>;

cchaos · 2022-03-30T20:18:35Z

...ed-ux-components/src/page_template/no_data_page/no_data_page_body/no_data_page_body.test.tsx

+  const actionCard = <div key={'action'}>{el}</div>;
+  test('render', () => {
+    const component = shallowWithIntl(
+      <NoDataPageBody solution="Elastic" docsLink="test" actionCard={actionCard} logo={'elastic'} />


Suggested change

<NoDataPageBody solution="Elastic" docsLink="test" actionCard={actionCard} logo={'elastic'} />

<NoDataPageBody solution="Analytics" docsLink="test" actionCard={actionCard} logo={'logoKibana'} />

cchaos · 2022-03-30T20:21:27Z

...c/page_template/no_data_page/no_data_page_body/__snapshots__/no_data_page_body.test.tsx.snap

+    <EuiSpacer
+      size="l"
+    />
+    <h1 />


Hm, looks like we should either be requiring a pageTitle in NoDataPageBody or ensuring an empty h1 is not rendered if there is no pageTitle.

cchaos · 2022-03-30T20:24:43Z

packages/kbn-shared-ux-components/src/page_template/no_data_page/no_data_page.tsx

+    <div {...rest}>
+      <NoDataPageBody
+        pageTitle={title}
+        actionCard={actionCard}
+        logo={logo}
+        solution={solution}
+        docsLink={docsLink}
+      />
+    </div>


To be honest, do we really need this component? All it's doing is some string creation and passing it along to the NoDataPageBody component that needs them to render properly anyway. Seems like it's an extra wrapper that is no longer necessary and we can just move all the title stuff directly into NoDataPageBody.

I hope you meant the other way around 😅 I removed the NoDataPageBody component, making it all part of NoDataPage component. This also fixed the issue with the title, as it turns out we were already setting a default title in there in case none was provided.

Yeah either way really. The NoDataPage at this time didn't have much in it

cchaos · 2022-03-30T20:26:32Z

...-shared-ux-components/src/page_template/no_data_page/no_data_page_body/no_data_page_body.tsx

+        </EuiTextColor>
+      </EuiText>
+      <EuiSpacer size="xxl" />
+      {actionCard}


We need to add margin-inline: auto to the card itself to ensure it gets centered horizontally.

majagrubic · 2022-03-31T08:42:13Z

@elasticmachine merge upstream

cchaos

LGTM. I did notice we have some issues in Storybook with custom styles and dark mode which I mentioned in the storybook slack channel.

I just had one extra suggestion for another control, but mergeable either way.

cchaos · 2022-03-31T13:56:22Z

packages/kbn-shared-ux-components/src/page_template/no_data_page/no_data_page.stories.tsx

+};
+
+PureComponent.argTypes = {
+  solution: {


Suggestion: Can we add a logo control that is blank by default?

kibana-ci · 2022-03-31T17:05:25Z

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

💚 Build #35222 succeeded fc2a50a
💔 Build #35180 failed adf83b1
💔 Build #35138 failed 167f966
💚 Build #34323 succeeded 04bde2a
💔 Build #34268 failed a35a3e1
💔 Build #34009 failed 4fb3975

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

kibanamachine · 2022-04-04T17:16:18Z

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 128372 or prevent reminders by adding the backport:skip label.

Maja Grubic added 19 commits March 18, 2022 11:00

[SharedUX] Migrate PageTemplate > NoDataPage > ActionCards

084afdf

[SharedUX] Migrate PageTemplate > NoDataPage > SolutionNav

e5c0b4c

Updating snapshot

5852696

[SharedUX] Migrate PageTemplate > NoDataPage

7e6ee56

Removing actionCards

97e2799

[SharedUX] Migrate PageTemplate > SolutionNavAvatar

4c02548

Fix comment

372840f

Merge branch 'page-template-3-2' into page-template-4

93b09e8

Renaming component

88fba5a

Fix folder structure

6b47699

Fix storybook

44c5039

Style fix

b65b050

Merge branch 'page-template-3-2' into page-template-4

a2ed0b5

Update so the component now uses the SolutionAvatar

5c4bfbf

Fix merge conflict

534fefa

Fix props

696006c

Remove unnecessary types files

fd72939

Fix failing test

efd4383

Updating failed test

7bd8409

kibanamachine and others added 4 commits March 28, 2022 07:29

Merge branch 'main' into page-template-4

42cbf0a

Merge branch 'main' into page-template-4

4fb3975

Merge branch 'main' into page-template-4

a35a3e1

Fix obsolete snapshot

04bde2a

majagrubic added Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) v8.2.0 labels Mar 29, 2022

majagrubic marked this pull request as ready for review March 29, 2022 07:56

majagrubic requested review from cchaos and a team March 29, 2022 07:56

majagrubic added the release_note:skip Skip the PR/issue when compiling release notes label Mar 29, 2022

majagrubic added v8.3.0 and removed v8.2.0 labels Mar 29, 2022

cchaos suggested changes Mar 30, 2022

View reviewed changes

Applying PR comments

e278b05

kibanamachine and others added 3 commits March 31, 2022 04:42

Merge branch 'main' into page-template-4

167f966

Update failing snapshot

adf83b1

Update failing snapshot

fc2a50a

majagrubic requested a review from cchaos March 31, 2022 13:20

cchaos approved these changes Mar 31, 2022

View reviewed changes

Add logo as an argtype

f6a40b3

majagrubic merged commit 79069d8 into elastic:main Mar 31, 2022

majagrubic deleted the page-template-4 branch March 31, 2022 17:09

kibanamachine added backport missing Added to PRs automatically when the are determined to be missing a backport. backport:skip This commit does not require backporting and removed backport missing Added to PRs automatically when the are determined to be missing a backport. labels Apr 4, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SharedUX] Migrate PageTemplate > NoData Page #128372

[SharedUX] Migrate PageTemplate > NoData Page #128372

majagrubic commented Mar 23, 2022 •

edited

Loading

majagrubic commented Mar 28, 2022

cchaos Mar 30, 2022

majagrubic Mar 31, 2022

cchaos Mar 30, 2022

cchaos Mar 30, 2022

cchaos Mar 30, 2022

majagrubic Mar 31, 2022 •

edited

Loading

cchaos Mar 31, 2022

cchaos Mar 30, 2022

majagrubic commented Mar 31, 2022

cchaos left a comment

cchaos Mar 31, 2022

majagrubic Mar 31, 2022

kibana-ci commented Mar 31, 2022

kibanamachine commented Apr 4, 2022

	<NoDataPageBody solution="Elastic" docsLink="test" actionCard={actionCard} logo={'elastic'} />
	<NoDataPageBody solution="Analytics" docsLink="test" actionCard={actionCard} logo={'logoKibana'} />

[SharedUX] Migrate PageTemplate > NoData Page #128372

[SharedUX] Migrate PageTemplate > NoData Page #128372

Conversation

majagrubic commented Mar 23, 2022 • edited Loading

Summary

Checklist

Risk Matrix

For maintainers

majagrubic commented Mar 28, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

majagrubic Mar 31, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

majagrubic commented Mar 31, 2022

cchaos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kibana-ci commented Mar 31, 2022

💚 Build Succeeded

Metrics [docs]

History

kibanamachine commented Apr 4, 2022

majagrubic commented Mar 23, 2022 •

edited

Loading

majagrubic Mar 31, 2022 •

edited

Loading