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

Update Analytics overview page to new empty state template #108532

Merged
merged 9 commits into from
Aug 16, 2021

Conversation

ryankeairns
Copy link
Contributor

@ryankeairns ryankeairns commented Aug 13, 2021

Relates to #107682

Summary

  • Replaces current Analytics empty state design with new version provided by KibanaPageTemplate
  • Removes files for old Analytics empty state view (getting_started)

Screenshot of Analytics overview empty state page

Note: the 'learn more' is pointing to https://www.elastic.co/guide/en/kibana/{VERSION}/index.html

Screen Shot 2021-08-13 at 9 12 33 AM

Checklist

Delete any items that are not applicable to this PR.
Covered by template PR

cc:/ @alexfrancoeur @VijayDoshi

@ryankeairns ryankeairns requested a review from a team as a code owner August 13, 2021 13:24
@ryankeairns ryankeairns requested a review from cchaos August 13, 2021 13:25
@ryankeairns ryankeairns added auto-backport Deprecated - use backport:version if exact versions are needed release_note:skip Skip the PR/issue when compiling release notes v7.15.0 v8.0.0 labels Aug 13, 2021
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.

Design changes LGTM. The view of this page is still dependent on fixing the logic based on the pre-installed beats/index-patterns (#108294). I tested this PR by manually deleting those pre-installed index-patterns.

Copy link
Member

@lukeelmers lukeelmers left a comment

Choose a reason for hiding this comment

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

Overall LGTM, just had a nit, a question, and a favor 🙂

href: addBasePath(`home#/tutorial_directory`),
},
},
docsLink: 'https://www.elastic.co/kibana/',
Copy link
Member

Choose a reason for hiding this comment

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

I know it's an extra step, but could we add this product page URL to the core docLinks service and import it from there instead?

We've done something similar for elastic.co/maps:

maps: `${ELASTIC_WEBSITE_URL}maps`,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Switched to a docs link (instead of the previous product link). This also meant returning the KIBANA_DOCS url since it points to the current version of the docs versus the other kibana links value that drops you on the full list of versions, which seems not very helpful.

Copy link
Member

Choose a reason for hiding this comment

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

Switched to a docs link (instead of the previous product link)

Ah, so if you want to use the docs link instead, then I don't think you'll need to change anything in the docLinks service itself, as this is already exposed under links.kibana:

kibana: `${KIBANA_DOCS}index.html`,

I think you would only need to update the service if you wanted to use the product link.

Copy link
Contributor

Choose a reason for hiding this comment

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

In the PR summary, it states that we want the learn more link to go to https://www.elastic.co/kibana/, so it's now using ${ELASTIC_WEBSITE_URL}kibana.

Copy link
Contributor Author

@ryankeairns ryankeairns Aug 15, 2021

Choose a reason for hiding this comment

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

This was my original guess not seeing guidance from Kibana Product, and then I thought docs might be better and aligned with what Obs is doing. @alexfrancoeur which of these do you prefer for the 'learn more' link?

  1. https://www.elastic.co/kibana/ or
  2. https://elastic.co/guide/en/kibana/{CURRENT_VERSION}/

Apologies if I missed this being answered elsewhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Discussed with Alex and we'd like to align with what other solutions are doing as we also feel the docs information is a better fit at this point in the experience. Security and Observability are also pointing the guide/docs.

@cchaos
Copy link
Contributor

cchaos commented Aug 14, 2021

@elasticmachine merge upstream

@ryankeairns ryankeairns requested a review from lukeelmers August 15, 2021 20:33
Copy link
Member

@lukeelmers lukeelmers left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for looking at the docLinks feedback.

@alexfrancoeur
Copy link

I haven't pulled the PR yet, so not sure if this is the latest but had a few quick questions. We'll want to make sure @AlonaNadler provides some input as well.

Product wise, do we want to be calling this page "Elastic Analytics" or just "Analytics"?

Second, the CTA is for beats but really where they're going includes beats tutorials, a link out to the integrations page, file upload and sample data. Is it possible to simplify and just say "Add Data"?

I don't think we need answers before FF, and can always adjust text before the 7.15 public release, but it'd be good to align on how we want to talk about this grouping of apps in Kibana.

@ryankeairns ryankeairns force-pushed the rk/analytics-empty-state branch from ef0dc76 to 3b55390 Compare August 16, 2021 14:36
@ryankeairns
Copy link
Contributor Author

Amended that last commit to remove the additions to the doc links service (🎩 Caroline). I was misinterpreting the value of the links.kibana URL which turned out to be doing what I was actually after.

@gchaps
Copy link
Contributor

gchaps commented Aug 16, 2021

I made comments in this issue re: copy.

@ryankeairns
Copy link
Contributor Author

I made comments in this issue re: copy.

Thanks Gail. Those are defined in the underlying template, so they should appear here once changed at that level.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
kibanaOverview 33 31 -2

Async chunks

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

id before after diff
kibanaOverview 21.5KB 18.3KB -3.2KB

History

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

@ryankeairns ryankeairns merged commit 36bba6f into elastic:master Aug 16, 2021
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Aug 16, 2021
…08532)

* Use empty state page template

* Remove unused translations

* Fixed snaps

* Use docLinks service

* Fix test

* Revert "Use docLinks service"

Use exisiting docLinks.ELASTIC_WEBSITE_URL instead

* Update learn more link and test

* fix test

Co-authored-by: cchaos <[email protected]>
Co-authored-by: Kibana Machine <[email protected]>
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
7.x

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Aug 16, 2021
…108773)

* Use empty state page template

* Remove unused translations

* Fixed snaps

* Use docLinks service

* Fix test

* Revert "Use docLinks service"

Use exisiting docLinks.ELASTIC_WEBSITE_URL instead

* Update learn more link and test

* fix test

Co-authored-by: cchaos <[email protected]>
Co-authored-by: Kibana Machine <[email protected]>

Co-authored-by: Ryan Keairns <[email protected]>
Co-authored-by: cchaos <[email protected]>
@cchaos cchaos mentioned this pull request Aug 17, 2021
12 tasks
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 v7.15.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants