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

[Serverless] Add panels to side nav #167774

Merged
merged 87 commits into from
Oct 18, 2023

Conversation

sebelga
Copy link
Contributor

@sebelga sebelga commented Oct 2, 2023

This PR add supports for "panels" in the serverless side navigation.

Notes for reviewer

  1. Sorry this PR got so big! 😔
  2. I created 2 separate issue to update the documentation and add test coverage for this PR. I decided to not touch any of the current tests (except snapshot updates) to make sure my refactors did not bring any regression
  3. The best way to test the panels is to use storybook: yarn storybook shared_ux

API

renderAs property

In order to indicate that a group in the side navigation should open a panel we use the renderAs property and set it to 'panelOpener'.

Tree definition

const navigationDefinitionWithPanel: ProjectNavigationDefinition<any> = {
  navigationTree: {
    body: [
      {
        type: 'navGroup',
        id: 'example_projet',
        title: 'Example project',
        icon: 'logoObservability',
        children: [
          {
            id: 'group:openpanel1',
            title: 'Node title',
            renderAs: 'panelOpener', // Add this 
            children: [
              ... // the rest is the same
            ],
          },

With UI components

<Navigation.Group id="group:settings" title="Open panel" renderAs="panelOpener"> {/* add "renderAs*/}
  <Navigation.Group id="group1">
    <Navigation.Item link="link1" title="Logs" />
    <Navigation.Item link="link2" title="Signals" withBadge />
    <Navigation.Item link="link3" title="Tracing" />
  </Navigation.Group>
  ...
</Navigation.Group>

Configure the content to render in the panel

There are 2 ways to configure the content that is rendered in the panel:

  • Generate automatically based on the tree definition
  • Providing a custom component based on the navigation node id that has been clicked

Generate automatically

To generate automatically the content of the panel we simply add nested groups with their children to the navigation tree.

{
  id: 'group1',
  title: 'Open panel',
  renderAs: 'panelOpener', // mark the  group nav node to open a panel
  children: [
    // Here we insert nested **GROUPS**
    {
      id: 'group1',
      title: 'Group 1', // Title is optional
      children: [
        {
          link: 'group:settings.logs',
          title: 'Logs',
        },
        {
          link: 'group:settings.signals',
          title: 'Signals',
          openInNewTab: true, // New prop to open the link in a new tab
        },
        {
          link: 'group:settings.tracing',
          title: 'Tracing',
          withBadge: true, // New prop to render a badge next to the label. Its text defaults to "Beta"
        },
      ],
    },
    {
      id: 'group2',
      title: 'Group 2',
      appendHorizontalRule: true, // New prop: add a separator after the group
      children: [
        {
          id: 'group2:settings.logs',
          link: 'group:settings.logs',
          title: 'Logs',
          withBadge: true,
          badgeOptions: { text: 'NEW' }, // Change the text of the badge
        },
       ...
      ],
    },
    // Groups with accordion
    {
      id: 'group3',
      title: 'MANAGEMENT',
      renderAs: 'accordion', // Mark the group as collapsible and render it inside an accordion
      children: [
        {
          id: 'group2-A',
          title: 'Group 1',
          children: [
            {
              link: 'group:settings.logs',
              title: 'Logs',
            },
            ...
          ],
        },
      }
  ],
},

Provide a custom component

If there is a need to render a custom component instead of the generated one we can pass a panelContentProvider

const panelContentProvider: ContentProvider = (id: string) => {
  if (id === 'some.node.id') {
    return; // Use the default, auto generated content
  }

  if (id === 'foo') {
    return {
      // Render a custom component in the panel
      content: ({
        selectedNode, // the node that trigger the opening of the panel
        activeNodes, // the current active nodes based on the current URL location
        closePanel, // handler to close the panel
      }) => {
        {/* return any JSX */}
        return <div>Hello</div>
      },
    };
  }

  if (id === 'bar') {
    // If you only need to customize the panel title, you can provide just the `title` prop
    // The rest will be auto generated
    return {
      title: 'Custom panel title',
      // Or with JSX
      // title: <div style={{ backgroundColor: 'yellow', fontWeight: 600 }}>My title</div>,
    };
  }
};

// The content provider can then be passed to the `DefaultNavigation` or `Navigation` components

<DefaultNavigation
  {...navigationTreeDefinition}
  panelContentProvider={panelContentProvider}
/>

Screenshots

Screenshot 2023-10-16 at 11 57 28 Screenshot 2023-10-16 at 11 57 52

@sebelga sebelga force-pushed the serverless-chrome/add-panel-to-side-nav branch from 547dc82 to 814fdea Compare October 9, 2023 11:35
Copy link
Member

@sphilipse sphilipse left a comment

Choose a reason for hiding this comment

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

search changes LGTM

Copy link
Contributor

@semd semd left a comment

Choose a reason for hiding this comment

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

Security Solution code LGTM

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.

great work 👏
I tested storybook, search and obit.
Also tried to explore sideNavStatus option for fixing some breadcrumbs, but there are some other things that need to be done first

);

const onIconClick = useCallback(() => {
openPanel(item);
Copy link
Contributor

Choose a reason for hiding this comment

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

should clicking the icon again close the panel?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interesting... I didn't think about that. I might not re-launch a full CI for that but I'm adding it to my current branch

Copy link
Contributor

@yngrdyn yngrdyn left a comment

Choose a reason for hiding this comment

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

APM changes LGTM

@sebelga sebelga enabled auto-merge (squash) October 16, 2023 18:06
@sebelga
Copy link
Contributor Author

sebelga commented Oct 16, 2023

@elasticmachine merge upstream

@sebelga sebelga merged commit 09bda6a into elastic:main Oct 18, 2023
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Security Solution Cypress Tests #10 / Changing alert status Opening alerts can mark a closed alert as open can mark a closed alert as open

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolutionServerless 470 480 +10
serverlessObservability 65 88 +23
serverlessSearch 167 190 +23
total +56

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/core-chrome-browser 70 69 -1
@kbn/shared-ux-chrome-navigation 38 48 +10
total +9

Async chunks

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

id before after diff
securitySolutionServerless 320.2KB 335.7KB +15.5KB
serverlessSearch 126.1KB 126.1KB +1.0B
total +15.5KB

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/core-chrome-browser 1 0 -1

Page load bundle

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

id before after diff
core 367.5KB 368.0KB +451.0B
securitySolutionServerless 47.2KB 47.3KB +114.0B
serverlessObservability 44.1KB 59.7KB +15.5KB
serverlessSearch 30.8KB 46.2KB +15.4KB
total +31.5KB
Unknown metric groups

API count

id before after diff
@kbn/core-chrome-browser 172 163 -9
@kbn/shared-ux-chrome-navigation 47 60 +13
total +4

History

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

@kibanamachine kibanamachine added v8.12.0 backport:skip This commit does not require backporting labels Oct 18, 2023
@sebelga sebelga deleted the serverless-chrome/add-panel-to-side-nav branch October 18, 2023 10:20
hop-dev pushed a commit to hop-dev/kibana that referenced this pull request Oct 18, 2023
@tsullivan
Copy link
Member

Great work, @sebelga !!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Project:Serverless Work as part of the Serverless project for its initial release release_note:skip Skip the PR/issue when compiling release notes Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) v8.12.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants