[Guided onboarding] Landing page updates #143194

yuliacech · 2022-10-12T13:00:10Z

Summary

This PR updates the Guided onboarding Landing page that can be found at /app/home#/getting_started. The Landing page has now 4 cards: 3 solution guides and a link to Infrastructure category. The guide card displays the state of each guide with the progress bar and a button to "start/continue" the guide.

Note: The "view integrations" button links to /app/integrations/browse/infrastructure but the category doesn't have any integrations added to it yet. As soon as there are any integrations in that category, the link should work as expected.

Screenshots

No guides started yet

The header button is disabled, guide cards have "view guide" buttons.

"View guide" button is clicked

The dropdown panel shows the selected guide and the user can start the first step. The header button is enabled.

A guide is in progress

The guide card displays the progress and the "continue" button.

A guide has been completed

The guide card displays the completed progress bar and the "view guide" button.

How to test

Enable the guided onboarding plugin by adding guidedOnboarding.ui: true to the file /config/kibana.dev.yml.
Start ES and Kibana with examples: yarn es snapshot and yarn start --run-examples.
Navigate to /app/home#/getting_started and check the landing page when no guides have been started yet.
Click the card for the search guide and check that the dropdown panel is being displayed and the first step can be started.
Start the first step and complete it on the page /app/guidedOnboardingExample/stepOne.
Navigate back to the landing page and check that the search card now displays the progress.
Complete the search guide and check that the landing page displays the search guide as completed.

Checklist

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Unit or functional tests were updated or added to match the most common scenarios

elasticmachine · 2022-10-12T17:08:08Z

Pinging @elastic/platform-onboarding (Team:Journey/Onboarding)

cindychangy

Thanks @yuliacech, this looks good! I left a couple of small style tweaks. They can move to the larger design polish task as well if preferred.

src/plugins/home/public/application/components/guided_onboarding/use_case_card.tsx

cindychangy · 2022-10-13T11:35:29Z

src/plugins/home/public/application/components/guided_onboarding/getting_started.tsx

@@ -86,15 +99,18 @@ export const GettingStarted = () => {
          </EuiText>
          <EuiSpacer size="s" />
          <EuiSpacer size="xxl" />
-          <EuiFlexGrid columns={3} gutterSize="xl">
+          <EuiFlexGrid columns={4} gutterSize="l">


I was unable to insert directly on line 97 but can we slightly modify the line to read: <EuiText size="m" textAlign="center">

…ng/use_case_card.tsx Co-authored-by: Cindy Chang <cindyisachang@gmail.com>

…-fix'

majagrubic

Thanx for extracting this into a package. I left some comments around the code and the home plugin specifcially.

majagrubic · 2022-10-19T13:44:27Z

packages/kbn-guided-onboarding/src/components/landing_page/guide_card_footer.tsx

+          )}
+        />
+        <EuiSpacer size="l" />
+        <div className="eui-textCenter">


What is this for? We shouldn't be using eui class names directly. Looks like you need a flexGroup?

I followed your suggestion to not use a flexGroup when there is only 1 component. Please let me know if there is a better way to center the button in the footer.

Sorry, maybe my comment wasn't clear initially. We shouldn't be using eui- classes directly in our React components, as they are internal to EUI and could possibly change. What I meant was - you can use EuiFlexGroup to achieve centering of the button (it's fine even for one component). Another way is to set the div width to 50% and margin to auto:
https://jsfiddle.net/79kjtwqo/1/

I don't think eui-textCenter is internal to EUI, or at least is is documented as an available utility class here - https://elastic.github.io/eui/#/theming/typography/utilities. I would argue it is OK to use here.

I agree with Alison here, if the EUI class should not be used in Kibana, there should be an eslint rule for that.

@majagrubic was absolutely correct here.

@alisonelizabeth it's not that it's internal to EUI, it's that they're likely not going to exist for much longer. Even on the documentation link you reference, it states:

For most of your usages we recommend using EuiText or EuiTitle components directly instead of these theme tokens. Or head to the text utilities page for helper classes and functions.

EUI is moving to Emotion:

EUI is transitioning to a CSS-in-JS solution for theming and requires consuming/customizing global variables in both the Sass and CSS-in-JS methods. To track this effort, subscribe to the Meta ticket.

Using these classes directly should be for edge cases only. The right solution for this would have been to use EuiText, rather than a div with a global EUI classname.

@yuliacech The reason there's no lint rule is that it would literally blow up the linter, due to the number of exceptions we've made in the past, (or older cases before Emotion and theming).

I would recommend a follow-up PR to switch to EuiText:

https://elastic.github.io/eui/#/display/text#alignment

There are two ways to align text. Either individually by applying EuiTextAlign on individual text objects, or by passing the textAlign prop directly on EuiText for a blanket approach across the entirety of your text.

@alisonelizabeth it's not that it's internal to EUI, it's that they're likely not going to exist for much longer.

👍 thanks for clarifying

The right solution for this would have been to use EuiText, rather than a div with a global EUI classname.

This is for centering a button though, right @yuliacech? I don't think EuiText is appropriate in that case. Perhaps @majagrubic earlier suggestion is the best approach:

What I meant was - you can use EuiFlexGroup to achieve centering of the button (it's fine even for one component). Another way is to set the div width to 50% and margin to auto:
https://jsfiddle.net/79kjtwqo/1/

I've opened #143794 to follow up on this.

Thanks a lot for the context, @clintandrewhall! I just wish this were communicated more broadly because with the number of Kibana contributors we won't be able to address the issue via individual code reviews. An eslint rule might be a grind at the beginning to disable all lines with classes, but at least it would prevent new ones from being added. Otherwise a documentation update might be an easier alternative. From the text, it's not clear that the classes will be deleted soon and currently should not be used in production, maybe a deprecation could be added there?

majagrubic · 2022-10-19T13:44:52Z

packages/kbn-guided-onboarding/src/components/landing_page/guide_card_footer.tsx

+            fill
+            onClick={() => activateGuide(useCase, guideState)}
+          >
+            {i18n.translate(


Can this be a constant as well?

majagrubic · 2022-10-19T13:45:04Z

packages/kbn-guided-onboarding/src/components/landing_page/guide_card_footer.tsx

+        value={numberCompleteSteps}
+        max={numberSteps}
+        size="s"
+        label={i18n.translate(


majagrubic · 2022-10-19T13:45:13Z

packages/kbn-guided-onboarding/src/components/landing_page/guide_card_footer.tsx

+        )}
+      />
+      <EuiSpacer size="l" />
+      <div className="eui-textCenter">


Same question as above

majagrubic · 2022-10-19T13:46:17Z

packages/kbn-guided-onboarding/src/components/landing_page/guide_card_footer.tsx

+import { GuideId, GuideState } from '../../types';
+import { UseCase } from './use_case_card';
+
+export const GuideCardFooter = ({


Looks like there are not tests for this component.

majagrubic · 2022-10-19T13:57:59Z

src/plugins/home/public/application/components/guided_onboarding/getting_started.tsx

+      setGuidesState(allGuides.state);
+    }
+  }, [guidedOnboardingService]);
+  useEffect(() => {


nit: please add space between useEffect and previous line.

majagrubic · 2022-10-19T13:58:40Z

src/plugins/home/public/application/components/guided_onboarding/getting_started.tsx

+  const isDarkTheme = uiSettings.get<boolean>('theme:darkMode');
+  const activateGuide = async (useCase: UseCase, guideState?: GuideState) => {
+    await guidedOnboardingService?.activateGuide(useCase as GuideId, guideState);
+    // TODO error handling


Let's do it now?

We don't have the correct error handling in the API yet, so this will be addressed in a separate issue

Could you link the issue in the TODO then?

majagrubic · 2022-10-19T14:00:13Z

src/plugins/home/public/application/components/guided_onboarding/getting_started.tsx

            </EuiFlexItem>
            <EuiFlexItem>
-              <UseCaseCard useCase="security" />
+              <GuideCard


Can we render these cards in a loop? It looks like (almost) all are the same

We need to display a guide card for search, a guide card for observability, a link card for observability and a guide card for security, so I don't think a loop would work with this logic.

Would something like this not work?

{ ['search', 'observability', 'observabilityLink', 'security'].map((useCase) => { if (useCase === 'observabilityLink') { return (<EuiFlexItem> <ObservabilityLinkCard .... /> </EuiFlexItem>) ; } return (<EuiFlexItem> <GuideCard useCase={useCase} .../> </EuiFlexItem>); }; }

yuliacech · 2022-10-19T15:05:03Z

Hi @majagrubic, thanks a lot for your review! I addressed your feedback in 0d46ae3, could you please have another look?

packages/kbn-guided-onboarding/src/components/landing_page/observability_link_card.tsx

kellyemurphy · 2022-10-19T16:26:50Z

Looking back at the design files, there are more differences in the text here.

(Plus "Search my Data" should be "Search my data", which you have correct in your PR)

…ervability_link_card.tsx Co-authored-by: Kelly Murphy <kelly.murphy@elastic.co>

yuliacech · 2022-10-19T16:55:26Z

Thanks a lot, @kellyemurphy! I used the figma file you linked and updated the texts, the screenshots in the PR description should be the latest state as well.

alisonelizabeth · 2022-10-19T19:37:25Z

@elasticmachine merge upstream

kibana-ci · 2022-10-20T09:06:06Z

💚 Build Succeeded

Buildkite Build
Commit: bbaeb90

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`home`	175	180	+5

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`@kbn/guided-onboarding`	-	17	+17
`guidedOnboarding`	19	36	+17
total			+34

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`home`	147.8KB	151.0KB	+3.2KB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id	before	after	diff
`@kbn/guided-onboarding`	-	2	+2
`guidedOnboarding`	3	1	-2
total			-0

Page load bundle

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

id	before	after	diff
`home`	10.8KB	10.8KB	+65.0B

Unknown metric groups

API count

id	before	after	diff
`@kbn/guided-onboarding`	-	17	+17
`guidedOnboarding`	19	36	+17
total			+34

History

💚 Build #81575 succeeded bee711c
💔 Build #81515 failed 64cb2ca
💔 Build #81346 failed b6eeb79
💔 Build #81332 failed 6ddd31b
💔 Build #81071 failed 6bcf31e
💔 Build #81042 failed 5f7b29e

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

majagrubic

Thanks for addressing the comments. Home plugin changes look good.

clintandrewhall

A few other comments while looking over this PR after the fact. YMMV. Feel free to reach out if Global Experience can help in any way!

clintandrewhall · 2022-10-20T18:53:01Z

packages/kbn-guided-onboarding/src/components/landing_page/guide_card_footer.tsx

+        // Used for FS tracking
+        data-test-subj={`onboarding--guideCard--view--${useCase}`}
+        fill
+        onClick={() => activateGuide(useCase, guideState)}


Incidentally, this is also a problem: you're creating a new function on every render cycle.

clintandrewhall · 2022-10-20T18:53:15Z

packages/kbn-guided-onboarding/src/components/landing_page/guide_card_footer.tsx

+          // Used for FS tracking
+          data-test-subj={`onboarding--guideCard--continue--${useCase}`}
+          fill
+          onClick={() => activateGuide(useCase, guideState)}


clintandrewhall · 2022-10-20T18:57:10Z

packages/kbn-guided-onboarding/src/components/landing_page/guide_card_footer.tsx

+  activateGuide: (useCase: UseCase, guideState?: GuideState) => void;
+}
+export const GuideCardFooter = ({ guides, useCase, activateGuide }: GuideCardFooterProps) => {
+  const guideState = guides.find((guide) => guide.guideId === (useCase as GuideId));


This feels like a code smell-- casting a UseCase as a GuideId, and that UseCase and GuideId are identical...?

clintandrewhall · 2022-10-20T18:59:00Z

packages/kbn-guided-onboarding/src/components/landing_page/use_case_card.tsx

+  },
+};
+
+export type UseCase = 'search' | 'observability' | 'security';


I'm curious why this type is necessary, when it matches GuideId. Appears to me these types need coalescing.

yuliacech and others added 4 commits October 11, 2022 19:12

[Guided onboarding] Updated landing page

eacfb87

[Guided onboarding] Finished landing page changes

a7812c6

Merge branch 'main' into guided_onboarding/8.6_landing_page

cced571

[Guided onboarding] Fixed card for completed guides

5a4cd7f

gchaps requested a review from kellyemurphy October 12, 2022 14:30

yuliacech and others added 2 commits October 12, 2022 19:05

[Guided onboarding] Fixed types errors

efce13f

Merge branch 'main' into guided_onboarding/8.6_landing_page

6d9e1f4

yuliacech marked this pull request as ready for review October 12, 2022 17:07

yuliacech requested a review from a team as a code owner October 12, 2022 17:07

yuliacech requested a review from a team October 12, 2022 17:07

yuliacech added release_note:skip Skip the PR/issue when compiling release notes Team:Journey/Onboarding Platform Journey Onboarding team v8.6.0 labels Oct 12, 2022

yuliacech requested review from alisonelizabeth and cindychangy October 12, 2022 17:08

yuliacech mentioned this pull request Oct 13, 2022

[Guided onboarding] Add Cloud check #143259

Closed

yuliacech and others added 2 commits October 13, 2022 10:47

[Guided onboarding] Fixed i18n issues

7870012

Merge branch 'main' into guided_onboarding/8.6_landing_page

2d5e83e

cindychangy approved these changes Oct 13, 2022

View reviewed changes

cindychangy reviewed Oct 13, 2022

View reviewed changes

src/plugins/home/public/application/components/guided_onboarding/use_case_card.tsx Outdated Show resolved Hide resolved

cindychangy reviewed Oct 13, 2022

View reviewed changes

src/plugins/home/public/application/components/guided_onboarding/use_case_card.tsx Outdated Show resolved Hide resolved

cindychangy reviewed Oct 13, 2022

View reviewed changes

src/plugins/home/public/application/components/guided_onboarding/use_case_card.tsx Outdated Show resolved Hide resolved

cindychangy reviewed Oct 13, 2022

View reviewed changes

yuliacech and others added 6 commits October 13, 2022 15:41

Merge branch 'main' into guided_onboarding/8.6_landing_page

3feaf96

Update src/plugins/home/public/application/components/guided_onboardi…

2f14aca

…ng/use_case_card.tsx Co-authored-by: Cindy Chang <cindyisachang@gmail.com>

Update src/plugins/home/public/application/components/guided_onboardi…

3d00f20

…ng/use_case_card.tsx Co-authored-by: Cindy Chang <cindyisachang@gmail.com>

Update src/plugins/home/public/application/components/guided_onboardi…

a35f49e

…ng/use_case_card.tsx Co-authored-by: Cindy Chang <cindyisachang@gmail.com>

[Guided onboarding] Added CR comments

493c6c6

[Guided onboarding] Added view guide button to the completed guide

c0d55e4

yuliacech and others added 7 commits October 18, 2022 17:14

[Guided onboarding] Update the tests for the api

6bcf31e

Merge branch 'main' into guided_onboarding/8.6_landing_page

5750826

[Guided onboarding] Fixed the i18n errors

79cc001

[Guided onboarding] Fixed the i18n errors

89793c5

[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…

6ddd31b

…-fix'

[Guided onboarding] Fixed the jest tests

b6eeb79

Merge branch 'main' into guided_onboarding/8.6_landing_page

cce77f7

majagrubic reviewed Oct 19, 2022

View reviewed changes

[Guided onboarding] Home changes

0d46ae3

kellyemurphy reviewed Oct 19, 2022

View reviewed changes

packages/kbn-guided-onboarding/src/components/landing_page/observability_link_card.tsx Outdated Show resolved Hide resolved

yuliacech mentioned this pull request Oct 19, 2022

[Guided onboarding] Create a manual test plan #143685

Closed

4 tasks

yuliacech and others added 2 commits October 19, 2022 18:31

Update packages/kbn-guided-onboarding/src/components/landing_page/obs…

c843e1c

…ervability_link_card.tsx Co-authored-by: Kelly Murphy <kelly.murphy@elastic.co>

[Guided onboarding] Address copy feedback

64cb2ca

kibanamachine and others added 3 commits October 19, 2022 13:37

Merge branch 'main' into guided_onboarding/8.6_landing_page

bee711c

[Guided onboarding] Address CR feedback

f340248

Merge branch 'main' into guided_onboarding/8.6_landing_page

bbaeb90

yuliacech enabled auto-merge (squash) October 20, 2022 08:09

majagrubic approved these changes Oct 20, 2022

View reviewed changes

yuliacech merged commit 9c0dd18 into elastic:main Oct 20, 2022

kibanamachine added the backport:skip This commit does not require backporting label Oct 20, 2022

alisonelizabeth mentioned this pull request Oct 20, 2022

[Guided onboarding] Fix card footer button markup #143794

Closed

clintandrewhall reviewed Oct 20, 2022

View reviewed changes

alisonelizabeth mentioned this pull request Oct 24, 2022

[Guided onboarding] [Observability] Add infrastructure card to landing page #140721

Closed

yuliacech deleted the guided_onboarding/8.6_landing_page branch November 22, 2022 09:31

[Guided onboarding] Landing page updates #143194

[Guided onboarding] Landing page updates #143194

Conversation

yuliacech commented Oct 12, 2022 • edited Loading

Summary

Screenshots

No guides started yet

"View guide" button is clicked

A guide is in progress

A guide has been completed

How to test

Checklist

elasticmachine commented Oct 12, 2022

cindychangy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

majagrubic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

majagrubic Oct 19, 2022 • edited Loading

Choose a reason for hiding this comment

alisonelizabeth Oct 19, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

clintandrewhall Oct 20, 2022 • edited Loading

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

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

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

yuliacech commented Oct 19, 2022

kellyemurphy commented Oct 19, 2022 • edited Loading

yuliacech commented Oct 19, 2022

alisonelizabeth commented Oct 19, 2022

kibana-ci commented Oct 20, 2022

💚 Build Succeeded

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Public APIs missing exports

Page load bundle

API count

History

majagrubic left a comment

Choose a reason for hiding this comment

clintandrewhall left a 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

Choose a reason for hiding this comment

yuliacech commented Oct 12, 2022 •

edited

Loading

majagrubic Oct 19, 2022 •

edited

Loading

alisonelizabeth Oct 19, 2022 •

edited

Loading

clintandrewhall Oct 20, 2022 •

edited

Loading

kellyemurphy commented Oct 19, 2022 •

edited

Loading