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

Kibana developer examples landing page #67049

Merged

Conversation

stacey-gammon
Copy link
Contributor

@stacey-gammon stacey-gammon commented May 19, 2020

Summary

Our developer examples were getting hard to navigate because so many of them show up in the app nav menu. Only a few of them are the "landing pages", while others exist to serve as part of the example.

A searchable landing page makes it easier to find examples and avoids cluttering the nav menu.

As always, run via yarn start --run-examples to load Kibana with developer examples!

Before:

A long list of apps make it difficult for developers to know where to go to learn about a service.

Screen Shot 2020-05-21 at 10 25 36 AM

After:

A single "developer examples" app is a landing page for all the registered examples:

navigation

Links to example plugins with descriptions, an optional image, and other helpful links. Images are not shown in this version due to a bug that will be fixed by #67561.

Screen Shot 2020-05-28 at 10 59 18 AM

This is what some of the images look like, but this is an older screenshot:

Screen Shot 2020-05-21 at 10 38 03 AM

It's searchable!

Screen Shot 2020-05-21 at 10 38 44 AM

Conclusion

The goals of this landing page are to make it easier for developers to find working, texted examples of services.

Unrelated code changes:

  • bfetch explorer was being run from the legacy platform for tests, which failed when it relied on another KP plugin (examples). I had to get rid of that and move the tests to the right config, so the example is running as a KP plugin, and the tests are inside tests/examples.
  • Because many links aren't in the nav anymore, switched the tests around to navigate to app, not click app menu item.

@stacey-gammon stacey-gammon force-pushed the 2020-05-14-examples-landing-page branch 2 times, most recently from 4e758a1 to 2349a9e Compare May 21, 2020 14:09
@stacey-gammon stacey-gammon changed the title Kibana developer portal Kibana developer examples landing page May 21, 2020
@stacey-gammon stacey-gammon force-pushed the 2020-05-14-examples-landing-page branch 6 times, most recently from 19b7d12 to 98a8b6b Compare May 26, 2020 22:08
@spalger
Copy link
Contributor

spalger commented May 26, 2020

@elasticmachine merge upstream

@spalger
Copy link
Contributor

spalger commented May 26, 2020

(jenkins upgrade required killing all jobs)

@stacey-gammon stacey-gammon force-pushed the 2020-05-14-examples-landing-page branch 3 times, most recently from c472350 to fb7fc46 Compare May 28, 2020 13:16
@stacey-gammon stacey-gammon marked this pull request as ready for review May 28, 2020 15:09
@stacey-gammon stacey-gammon requested a review from a team as a code owner May 28, 2020 15:09
@stacey-gammon stacey-gammon added release_note:skip Skip the PR/issue when compiling release notes v7.9.0 v8.0.0 labels May 28, 2020
@stacey-gammon stacey-gammon force-pushed the 2020-05-14-examples-landing-page branch from 6c05925 to fa2b9e6 Compare June 1, 2020 19:29
@Dosant
Copy link
Contributor

Dosant commented Jun 2, 2020

Played a bit, really nice! Also images make it a lot more lively 👍 (They are working in latest version!)

Some nits I have:

  1. Should we make the whole card clickable and as a link? EuiCard supports that. Having it as proper link would also make a right click work. (🤔 I guess I see why it isn't like this right now, probably this would be possible after add RedirectAppLinks component in kibana_react #67595)

  2. Looking at how long the descriptions are, I think we should make them left aligned and add a markdown support there. Markdown would be especially useful for state management description where we have functions listed createStateContainerReactHelpers. We have a component for markdown rendering: https://github.com/elastic/kibana/tree/master/src/plugins/kibana_react/public/markdown

  3. noopener & noreferer on target="_blank" links

@stacey-gammon
Copy link
Contributor Author

stacey-gammon commented Jun 2, 2020

Could we make the whole card clickable and as a link? EuiCard supports that.

I had it like that originally but it seemed to cause issues with nested links, like I'd click on the github link to the README and it would navigate me there, but also trigger the link on the Card. Also, much more minor, I didn't like the hover effect.

Having it as proper link would also make a right click work. (🤔 I guess I see why it isn't like this right now, probably this would be possible after #67595)

I was waiting on elastic/eui#1668 to make this work with the EUI component I used. Using both onClick and href we can do that trick we did with drilldowns and have onClick use navigateToApp to avoid the full page refresh.

Looking at how long the descriptions are, I think we should make them left aligned and add a markdown support there. Markdown would be especially useful for state management description where we have functions listed createStateContainerReactHelpers. We have a component for markdown rendering: https://github.com/elastic/kibana/tree/master/src/plugins/kibana_react/public/markdown

Interesting I had no idea we had a markdown component. I originally supported passing a string or just any react node to this card, but then I lost my very easy ability to search the description text, so I opted for text only. Better for searching, poorer for nice display. However, I don't think we really want a ton of text there. Essentially, it's for searching. Then the longer, more organized content, should be rendered in the landing page of the example.

noopener & noreferer on target="_blank" links

You mean with where I define the list links?

      links: [
        {
          label: 'README',
          href: 'https://github.com/elastic/kibana/blob/master/src/plugins/bfetch/README.md',
          iconType: 'logoGithub',
          size: 's',
          target: '_blank',
        },
      ],

?

Are you sure I need to add that there? If it internally uses EuiLink I think maybe not because I don't see those used in any EuiLink.

Or do you mean add that somehow here?: onClick={() => window.open(getUrlForApp(def.appId), '_blank')}?

@Dosant
Copy link
Contributor

Dosant commented Jun 2, 2020

@stacey-gammon,

I had it like that originally but it seemed to cause issues with nested links, like I'd click on the github link to the README and it would navigate me there, but also trigger the link on the Card.

Have you tried to stop propagation in internal link handler?
Yee, this isn't a blocker, just shared impressions.

Are you sure I need to add that there? If it internally uses EuiLink I think maybe not because I don't see those used in any EuiLink.

Looks like internally EuiLink indeed handles it, https://github.com/elastic/eui/blob/master/src/components/link/link.tsx#L140

Or do you mean add that somehow here?: onClick={() => window.open(getUrlForApp(def.appId), '_blank')}?

But if we do navigation ourselves, like in this case, then ideally we should handle it when calling window.open()

@stacey-gammon
Copy link
Contributor Author

Ready for another review @elastic/kibana-app-arch

@stacey-gammon
Copy link
Contributor Author

Ping again 🙏

@kobelb kobelb self-requested a review June 5, 2020 16:07
Copy link
Contributor

@kobelb kobelb left a comment

Choose a reason for hiding this comment

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

I really like these changes! Once we have support for plugins in folders, I could see us taking advantage of this here as well.

nit: I don't see examples/bfetch_explorer/public/bfetch.png being used anywhere... should we delete it?

Copy link
Contributor

@Dosant Dosant left a comment

Choose a reason for hiding this comment

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

LGTM!

I will follow up with cleaning up state management demo apps and grouping it under one section

@stacey-gammon
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@stacey-gammon stacey-gammon merged commit f89e911 into elastic:master Jun 8, 2020
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jun 8, 2020
* master:
  [ML] DFAnalytics results: ensure ml result fields are shown in data grid (elastic#68305)
  [security_solution] enable react-hooks/exhaustive-deps (elastic#68470)
  Closes elastic#66867 by adding missing, requried API params (elastic#68465)
  [Telemetry] collect number of visualization saved in the past 7, 30 and 90 days (elastic#67865)
  [Logs UI] View in context tweaks (elastic#67777)
  Kibana developer examples landing page (elastic#67049)
  Bump decompress package version (elastic#68386)
  fix elastic#66185 (elastic#66186)
  Bump pdfmake package version (elastic#68395)
  Unskip embeddables/adding_children suite (elastic#68111)
  Add embed mode options in the Share UI (elastic#58435)
  Adding key to avoid react warning (elastic#68491)
stacey-gammon added a commit to stacey-gammon/kibana that referenced this pull request Jun 9, 2020
* Kibana developer examples

* Batch explorer tests should be run in examples config

* Fix tests

* add codeowner for new developer examples plugin & readme cleanup

* Try to frame embeddable wording based on what a developer's goals are.

* Add noopener noreferer, fix bad merge

* Remove bfetch.png

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
# Conflicts:
#	.github/CODEOWNERS
stacey-gammon added a commit to stacey-gammon/kibana that referenced this pull request Jun 9, 2020
* Kibana developer examples

* Batch explorer tests should be run in examples config

* Fix tests

* add codeowner for new developer examples plugin & readme cleanup

* Try to frame embeddable wording based on what a developer's goals are.

* Add noopener noreferer, fix bad merge

* Remove bfetch.png

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
# Conflicts:
#	.github/CODEOWNERS
stacey-gammon added a commit that referenced this pull request Jun 10, 2020
* Kibana developer examples

* Batch explorer tests should be run in examples config

* Fix tests

* add codeowner for new developer examples plugin & readme cleanup

* Try to frame embeddable wording based on what a developer's goals are.

* Add noopener noreferer, fix bad merge

* Remove bfetch.png

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
# Conflicts:
#	.github/CODEOWNERS
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes review v7.9.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants