[Enterprise Search] Set up initial KibanaPageTemplate

[cee-chen]

Summary

Despite my best attempts to make this a smaller PR, it's still on the larger side and should definitely be reviewed commit-by-commit.

• High-level overview of the KibanaPageTemplate migration and why we're doing it/what we get out of it:
  • Visual consistency with other plugins/teams on Kibana (primarily around the solution nav and page headers)
  • Out of the box mobile responsiveness - we can (eventually) delete all our custom nav CSS/logic etc, including breakpoints, is mobile open state, a11y, etc. 🎉
  • Fixes that really annoying bug where if the telemetry callout is at the top of the page, the entire nav gets incorrectly offset (my personal favorite item on this list)
  • Per-view flexibility - we actually get even more flexibility to change certain layout props this way (e.g. per-page layout classes, if needed, as well as everything in EuiPageTemplate's props
  • Simpler & DRYer API - going forward, we won't need to manually include FlashMessages per-view, and things like page chrome & telemetry can now quickly be passed as simple props instead of requiring a component import
• What this PR does:
  • Sets up a shared <EnterpriseSearchPageTemplate /> which will replace the old <Layout /> component and provides some nicer/DRYer APIs for things like a loading state, empty state, flash messages, and a built-in read only mode callout
  • Sets up product-specific <AppSearchPageTemplate /> and <WorkplaceSearchPageTemplate /> components which manage their own solution navs and provide some nicer/DRYer APIs for things like page chrome and page telemetry
  • Updates several navigation/routing helpers to work with plain objects (used by EuiSideNav items) instead of only React components
  • Adds some new test_helpers for inspecting nested page headers within page templates
  • Updates only the Users & Roles view to use the new page template
    • Why this view? Because it's one shared by both AS & WS and made for a pretty nice example of how views get cleaned/DRY'ed out with the new page template, and I figured @scottybollinger might as well get this now during his Role Mappings work to prevent future merge conflicts
• What this PR does NOT do:
  • Affect any view that's not the Users & Roles / Role Mappings view
  • Set up any sub/nested navigation - that will be in future PRs
  • Delete any of the current custom Layout or SideNav components - that will be its own separate PR at the end of everything to allow granularly migrating batches of views at a time instead of all at once

Example code of what all views should generally look like, going forward:

export const SomePage: React.FC = () => {
  return (
    <AppSearchPageTemplate
      pageChrome={['Engines', 'some-engine', 'Some page']} // automatically sets AS breadcrumbs & page title
      pageViewTelemetry="some_page" // automatically fires a telemetry viewed event
      pageHeader={{
        pageTitle: 'Some page',
        description: <p>What the page does</p>,
        rightSideItems: [<EuiButton />],
      }}
    >
       Flash messages and read only mode state comes for free!
    </AppSearchPageTemplate>
  )
}

export const AnotherPage: React.FC = () => {
  const { dataLoading, hasItems } = useValues(SomeLogic);

  return (
    <WorkplaceSearchPageTemplate
      pageHeader={{ pageTitle: 'Another page' }}
      isLoading={dataLoading} // automatically renders a Loading component in place of the child content
      isEmptyState={!hasItems}
      emptyState={<EmptyState />} // e.g. some custom EuiCallOut - only renders in place of child content if isEmptyState is true
    >
       Non-empty content goes here!
    </AppSearchPageTemplate>
  )
}

Screenshots

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
• 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)
• This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
• This was checked for cross-browser compatibility

[cee-chen]

@elasticmachine merge upstream

[scottybollinger]

@constancecchen I haven't pushed it up yet but deleting the role_mappings_heading.tsx component is going to break my feature. Both sections are identical and share the same heading. The design calls for identical sections and I opted to not have a EuiPageHeader on this page for that reason.

[cee-chen]

Oooh, thanks Scotty, I totally missed that there's multiple headers on the same page. Super good call, let me rethink this and potentially run this by the EUI team for ideas.

FWIW there is a semantic and accessibility cost to losing the page header - your heading hierarchy gets messed up (e.g., no single parent h1 on the page) so we should definitely think carefully about changing that pattern (I assume that's the point of the KibanaPageTemplate).

That being said it's also totally possible to just pass in custom rendered JSX to pageHeader so I can definitely just do that if needed.

[byronhulcher]

I'm rubber-stamping this so that it can be merged after @elastic/workplace-search-frontend's review + approval since we are lacking @elastic/app-search-frontend reviewers until next Monday. I have not reviewed or QA'd this code. This was discussed in a recent App Search sync with @richkuz. I trust @constancecchen has represented the interests of the App Search team on this PR.

@constancecchen did me a solid and reviewed another PR of mine before I went away on PTO so I did a high level review and QA below and re-affirmed this approval

[cee-chen]

@byronhulcher If you have time after your Process Crawls work today to take a quick look at the API code example in the PR description and leave any developer experience feedback/requests that definitely would be appreciated, but if not please feel free to come back to this after your PTO and I can make any change requests retroactively/in a later PR 👍

[cee-chen]

Also, no idea what's going on with CI but those files aren't related to this PR 👊

[cee-chen]

@elasticmachine merge upstream

[cee-chen]

Apologies for the force push, in light of Scotty's comment I thought it best to start over on the Role Mappings conversion and limit the amount of diffs in that commit. @scottybollinger take a look at 4b1d526 and let me know what you think! Here's new screenshots of the views:

I think the only change in there now that I'm not sure will conflict with your current work is me removing the ProductName type union. If you really need that LMK and I'll add it back in, but specify our already i18n'd product name constants.

[byronhulcher]

I did a quick review of this and also poked around locally. Code looks good and I didn't notice anything broken in QA. I left some non-blocking comments.

[byronhulcher]

Should the filename of be closer to the exported component's name?

[cee-chen]

I don't feel super strongly either way, since this is namespaced (folder-wise... folder-spaced?) to app_search/ I don't think it's too confusing, but I know there's IDE implications to file-finding or showing tab names 🤷

FWIW I was kinda copying Workplace Search's folder architecture pattern here (they already have a layout/nav.tsx and layout/kibana_header_actions.tsx which we also now have our own version of), so I thought layout/page_template.tsx made sense as well 🤔

Definitely doesn't bother me either way though, so happy to revisit later if it's annoying down the road or if we want to circle back and do a vote

[byronhulcher]

Should this interface have a more specific name?

[cee-chen]

I've been using more generic type names (e.g. Props, Params) if they're not getting exported or reused externally. If they're used in other files, that's when I namespace them (e.g. PageTemplateProps, ReactRouterProps).

But otherwise it's to me it's kinda like not having to namespace isDocumentsLoading vs isAnalyticsLoading - the isLoading var name is scoped to the file or view, so it doesn't need to be overly specific.

The day I ever start using generic single letter variable names is the day you can hit me over the head with a shovel tho, gotta draw that line somewhere

[byronhulcher]

What's the plan for this TODO?

[cee-chen]

Upcoming PR!

[byronhulcher]

+1000000000000 for this comment, you love to see it

[byronhulcher]

I'd prefer an absolute path if we can use one for this, I think this would read a lot better as src/plugins/kibana_react/public/ as if it was a yarn installed third-party module

[cee-chen]

+++, but I don't think we can sadly :( I copied this from the Observability folks and AFAIK Kibana's aliases mostly involve src/core and not src/plugins. Definitely would love to make that request though - I might ping the DevOps folks in a little bit!

[cchaos]

😻 Thanks for implementing this. I love your writeup and glad to see that this effort opens your application up for even more possibilities/flexibilities.

One quick thing about the side-nav. The pattern is to not have those root level items actually be clickable/navigable. Instead we want to be consistent throughout and keep them solely to describe sections. I couldn't completely deter this functionality in EUI because it would break a lot of existing implementations but you can work around it by simply creating an empty first item like this:

items = [
  {
    name: '',
    items: [
      {
        name: 'Item 1',
        id: '',
      },
      ...etc
    ]
  }
]

[cee-chen]

Ahhh I was wondering about that Caroline!! I just assumed that's how it was supposed to work, LOL. Probably should have pinged you at some point 🤦‍♀️ Awesome, testing this out now. Just to check, is id: '' okay as well (since the ID prop is required?)

[scottybollinger]

LGTM! Thanks for the great comments and test helpers! 🎉

[spalger]

jenkins, test this

(restarting due to jenkins upgrade)

[cee-chen]

Thanks Spencer!! 🎊

[kibanamachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: b747586
• Storybooks Preview
• Documentation Changes

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
enterpriseSearch	1438	1454	+16

Async chunks

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

id	before	after	diff
enterpriseSearch	2.1MB	2.1MB	+7.2KB

Page load bundle

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

id	before	after	diff
enterpriseSearch	14.3KB	14.5KB	+238.0B

History

• 💔 Build #131772 failed fb0be5d
• 💚 Build #131462 succeeded 4b1d526
• 💚 Build #131421 succeeded 77c72ef
• 💔 Build #131392 failed 7a45629
• 💔 Build #131359 failed dbab179

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

[kibanamachine]

💚 Backport successful

Status	Branch	Result
✅	7.x

This backport PR will be merged automatically after passing CI.