-
Notifications
You must be signed in to change notification settings - Fork 8.1k
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
[Guided onboarding] Update observability tour #143006
[Guided onboarding] Update observability tour #143006
Conversation
…ng/observability_tour
…ng/observability_tour
…ng/observability_tour
…-ref HEAD~1..HEAD --fix'
await browser.removeLocalStorageItem(observTourStepStorageKey); | ||
}); | ||
|
||
describe('Tour enabled', () => { | ||
beforeEach(async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if it makes sense to keep these tests around once #138568 is implemented, but thought it would be good to maintain them for now until we have a clearer picture of what our E2E testing looks like.
Pinging @elastic/platform-onboarding (Team:Journey/Onboarding) |
Pinging @elastic/apm-ui (Team:APM) |
Pinging @elastic/uptime (Team:uptime) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import formatting changes in x-pack/plugins/security/public/management/roles/edit_role/privileges/kibana/feature_table/feature_table.tsx
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot, @alisonelizabeth!
Tested locally and the tour works as expected, code changes LGTM too 🎉
@elasticmachine merge upstream |
@elasticmachine merge upstream |
x-pack/plugins/infra/kibana.json
Outdated
@@ -16,7 +16,8 @@ | |||
"observability", | |||
"ruleRegistry", | |||
"unifiedSearch", | |||
"lens" | |||
"lens", | |||
"guidedOnboarding" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you help me understand why this would have to become a dependency of the infra
plugin?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@weltenwort Happy path, the EuiTour steps render on the Overview page. However, if a user navigates away via the left nav during the tour, the tour steps should still render (e.g., to the “Stream” page). The tour logic lives within the shared Observability page template, and is dependent on the guidedOnboarding
plugin.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, thanks for the explanation. But wouldn't the existing dependency on the observability
plugin (through which the infra
plugin renders the observability page template in the first place) already extend to the guidedOnboarding
dependency transitively?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@weltenwort I went back and looked through the code, and I was able to inject it as a dependency when createLazyObservabilityPageTemplate
is called. This should remove the need to make it a dependency of each plugin. Thanks for calling this out!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's great, thanks for the thoroughness!
…ng/observability_tour
…ng/observability_tour
@alisonelizabeth can we change the text on the final step of the setup guide? It shouldn't be a bullet point if there's only one and I think the text would read better as |
Thanks @kellyemurphy! I've updated the copy (f8f28b4). The bullet point issue will be addressed via #143606. |
💚 Build Succeeded
Metrics [docs]Public APIs missing comments
Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: |
This PR updates the Observability product tour implemented in 8.4 to work with the new guided onboarding framework. The tour will now display as part of step 3 of the Kubernetes guide.
Fixes #140656
How to test
kibana.dev.yml
file:guidedOnboarding.ui: true
yarn start --run-examples
/app/guidedOnboardingExample
Note: There is a bug right now when clicking on other links in the side nav with the tour in progress. This appears to be a regression introduced in the
KibanaPageTemplate
. I opened #143114 to track.Demo
observ_tour.mp4